From 2adba0d8e0ec22555cd115f5958cbf4c9ec19af3 Mon Sep 17 00:00:00 2001 From: Steve Yegge Date: Fri, 5 Dec 2025 16:20:43 -0800 Subject: [PATCH] feat(tombstone): implement delete-to-tombstone and TTL expiration (bd-3b4, bd-olt) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Phase 1 of tombstone migration: bd delete now creates tombstones instead of hard-deleting issues. Key changes: - Add CreateTombstone() method to SQLiteStorage for soft-delete - Modify executeDelete() to create tombstones instead of removing rows - Add IsExpired() method with 30-day default TTL and clock skew grace - Fix deleted_at schema from TEXT to DATETIME for proper time scanning - Update delete.go to call CreateTombstone (single issue path) - Still writes to deletions.jsonl for backward compatibility (dual-write) - Dependencies are removed when creating tombstones - Tombstones are excluded from normal searches (bd-1bu) TTL constants: - DefaultTombstoneTTL: 30 days - MinTombstoneTTL: 7 days (safety floor) - ClockSkewGrace: 1 hour 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude --- cmd/bd/delete.go | 33 +- internal/storage/sqlite/delete_test.go | 52 +-- .../migrations/018_tombstone_columns.go | 2 +- internal/storage/sqlite/queries.go | 163 ++++++++- internal/storage/sqlite/schema.go | 2 +- internal/storage/sqlite/tombstone_test.go | 332 ++++++++++++++++++ internal/types/types.go | 36 ++ internal/types/types_test.go | 207 +++++++++++ 8 files changed, 767 insertions(+), 60 deletions(-) create mode 100644 internal/storage/sqlite/tombstone_test.go diff --git a/cmd/bd/delete.go b/cmd/bd/delete.go index ca68f321..bb64b800 100644 --- a/cmd/bd/delete.go +++ b/cmd/bd/delete.go @@ -227,15 +227,13 @@ Force: Delete and orphan dependents inboundRemoved++ } } - // 4. Delete the issue itself from database - if err := deleteIssue(ctx, issueID); err != nil { - fmt.Fprintf(os.Stderr, "Error deleting issue: %v\n", err) + // 4. Create tombstone (instead of deleting from database) + // Phase 1 dual-write: still writes to deletions.jsonl (step 0), now also creates tombstone + if err := createTombstone(ctx, issueID, deleteActor, "manual delete"); err != nil { + fmt.Fprintf(os.Stderr, "Error creating tombstone: %v\n", err) os.Exit(1) } - // 5. Remove from JSONL (auto-flush can't see deletions) - if err := removeIssueFromJSONL(issueID); err != nil { - fmt.Fprintf(os.Stderr, "Warning: Failed to remove from JSONL: %v\n", err) - } + // Note: No longer call removeIssueFromJSONL - tombstone will be exported to JSONL // Schedule auto-flush to update neighbors markDirtyAndScheduleFlush() totalDepsRemoved := outgoingRemoved + inboundRemoved @@ -253,6 +251,20 @@ Force: Delete and orphan dependents } }, } +// createTombstone converts an issue to a tombstone record +// Note: This is a direct database operation since Storage interface doesn't have CreateTombstone +func createTombstone(ctx context.Context, issueID string, actor string, reason string) error { + // We need to access the SQLite storage directly + // Check if store is SQLite storage + type tombstoner interface { + CreateTombstone(ctx context.Context, id string, actor string, reason string) error + } + if t, ok := store.(tombstoner); ok { + return t.CreateTombstone(ctx, issueID, actor, reason) + } + return fmt.Errorf("tombstone operation not supported by this storage backend") +} + // deleteIssue removes an issue from the database // Note: This is a direct database operation since Storage interface doesn't have Delete func deleteIssue(ctx context.Context, issueID string) error { @@ -443,12 +455,7 @@ func deleteBatch(_ *cobra.Command, issueIDs []string, force bool, dryRun bool, c } // Update text references in connected issues (using pre-collected issues) updatedCount := updateTextReferencesInIssues(ctx, issueIDs, connectedIssues) - // Remove from JSONL - for _, id := range issueIDs { - if err := removeIssueFromJSONL(id); err != nil { - fmt.Fprintf(os.Stderr, "Warning: Failed to remove %s from JSONL: %v\n", id, err) - } - } + // Note: No longer remove from JSONL - tombstones will be exported to JSONL (bd-3b4) // Schedule auto-flush markDirtyAndScheduleFlush() // Output results diff --git a/internal/storage/sqlite/delete_test.go b/internal/storage/sqlite/delete_test.go index 59f933a9..c2c33c02 100644 --- a/internal/storage/sqlite/delete_test.go +++ b/internal/storage/sqlite/delete_test.go @@ -46,12 +46,12 @@ func TestDeleteIssues(t *testing.T) { t.Run("delete with cascade - should delete all dependents", func(t *testing.T) { store := newTestStore(t, "file::memory:?mode=memory&cache=private") - + // Create chain: bd-1 -> bd-2 -> bd-3 issue1 := &types.Issue{ID: "bd-1", Title: "Cascade Parent", Status: types.StatusOpen, Priority: 1, IssueType: types.TypeTask} issue2 := &types.Issue{ID: "bd-2", Title: "Cascade Child", Status: types.StatusOpen, Priority: 1, IssueType: types.TypeTask} issue3 := &types.Issue{ID: "bd-3", Title: "Cascade Grandchild", Status: types.StatusOpen, Priority: 1, IssueType: types.TypeTask} - + if err := store.CreateIssue(ctx, issue1, "test"); err != nil { t.Fatalf("Failed to create issue1: %v", err) } @@ -61,7 +61,7 @@ func TestDeleteIssues(t *testing.T) { if err := store.CreateIssue(ctx, issue3, "test"); err != nil { t.Fatalf("Failed to create issue3: %v", err) } - + dep1 := &types.Dependency{IssueID: "bd-2", DependsOnID: "bd-1", Type: types.DepBlocks} if err := store.AddDependency(ctx, dep1, "test"); err != nil { t.Fatalf("Failed to add dependency: %v", err) @@ -79,26 +79,26 @@ func TestDeleteIssues(t *testing.T) { t.Errorf("Expected 3 deletions (cascade), got %d", result.DeletedCount) } - // Verify all deleted - if issue, _ := store.GetIssue(ctx, "bd-1"); issue != nil { - t.Error("bd-1 should be deleted") + // Verify all converted to tombstones (bd-3b4) + if issue, _ := store.GetIssue(ctx, "bd-1"); issue == nil || issue.Status != types.StatusTombstone { + t.Error("bd-1 should be tombstone") } - if issue, _ := store.GetIssue(ctx, "bd-2"); issue != nil { - t.Error("bd-2 should be deleted") + if issue, _ := store.GetIssue(ctx, "bd-2"); issue == nil || issue.Status != types.StatusTombstone { + t.Error("bd-2 should be tombstone") } - if issue, _ := store.GetIssue(ctx, "bd-3"); issue != nil { - t.Error("bd-3 should be deleted") + if issue, _ := store.GetIssue(ctx, "bd-3"); issue == nil || issue.Status != types.StatusTombstone { + t.Error("bd-3 should be tombstone") } }) t.Run("delete with force - should orphan dependents", func(t *testing.T) { store := newTestStore(t, "file::memory:?mode=memory&cache=private") - + // Create chain: bd-1 -> bd-2 -> bd-3 issue1 := &types.Issue{ID: "bd-1", Title: "Force Parent", Status: types.StatusOpen, Priority: 1, IssueType: types.TypeTask} issue2 := &types.Issue{ID: "bd-2", Title: "Force Child", Status: types.StatusOpen, Priority: 1, IssueType: types.TypeTask} issue3 := &types.Issue{ID: "bd-3", Title: "Force Grandchild", Status: types.StatusOpen, Priority: 1, IssueType: types.TypeTask} - + if err := store.CreateIssue(ctx, issue1, "test"); err != nil { t.Fatalf("Failed to create issue1: %v", err) } @@ -108,7 +108,7 @@ func TestDeleteIssues(t *testing.T) { if err := store.CreateIssue(ctx, issue3, "test"); err != nil { t.Fatalf("Failed to create issue3: %v", err) } - + dep1 := &types.Dependency{IssueID: "bd-2", DependsOnID: "bd-1", Type: types.DepBlocks} if err := store.AddDependency(ctx, dep1, "test"); err != nil { t.Fatalf("Failed to add dependency: %v", err) @@ -129,15 +129,15 @@ func TestDeleteIssues(t *testing.T) { t.Errorf("Expected bd-2 to be orphaned, got %v", result.OrphanedIssues) } - // Verify bd-1 deleted, bd-2 and bd-3 still exist - if issue, _ := store.GetIssue(ctx, "bd-1"); issue != nil { - t.Error("bd-1 should be deleted") + // Verify bd-1 is tombstone, bd-2 and bd-3 still active (bd-3b4) + if issue, _ := store.GetIssue(ctx, "bd-1"); issue == nil || issue.Status != types.StatusTombstone { + t.Error("bd-1 should be tombstone") } - if issue, _ := store.GetIssue(ctx, "bd-2"); issue == nil { - t.Error("bd-2 should still exist") + if issue, _ := store.GetIssue(ctx, "bd-2"); issue == nil || issue.Status == types.StatusTombstone { + t.Error("bd-2 should still be active") } - if issue, _ := store.GetIssue(ctx, "bd-3"); issue == nil { - t.Error("bd-3 should still exist") + if issue, _ := store.GetIssue(ctx, "bd-3"); issue == nil || issue.Status == types.StatusTombstone { + t.Error("bd-3 should still be active") } }) @@ -175,7 +175,7 @@ func TestDeleteIssues(t *testing.T) { t.Run("delete multiple issues at once", func(t *testing.T) { store := newTestStore(t, "file::memory:?mode=memory&cache=private") - + independent1 := &types.Issue{ID: "bd-10", Title: "Independent 1", Status: types.StatusOpen, Priority: 1, IssueType: types.TypeTask} independent2 := &types.Issue{ID: "bd-11", Title: "Independent 2", Status: types.StatusOpen, Priority: 1, IssueType: types.TypeTask} @@ -194,12 +194,12 @@ func TestDeleteIssues(t *testing.T) { t.Errorf("Expected 2 deletions, got %d", result.DeletedCount) } - // Verify both deleted - if issue, _ := store.GetIssue(ctx, "bd-10"); issue != nil { - t.Error("bd-10 should be deleted") + // Verify both converted to tombstones (bd-3b4) + if issue, _ := store.GetIssue(ctx, "bd-10"); issue == nil || issue.Status != types.StatusTombstone { + t.Error("bd-10 should be tombstone") } - if issue, _ := store.GetIssue(ctx, "bd-11"); issue != nil { - t.Error("bd-11 should be deleted") + if issue, _ := store.GetIssue(ctx, "bd-11"); issue == nil || issue.Status != types.StatusTombstone { + t.Error("bd-11 should be tombstone") } }) } diff --git a/internal/storage/sqlite/migrations/018_tombstone_columns.go b/internal/storage/sqlite/migrations/018_tombstone_columns.go index 32711f75..58f2f2b7 100644 --- a/internal/storage/sqlite/migrations/018_tombstone_columns.go +++ b/internal/storage/sqlite/migrations/018_tombstone_columns.go @@ -16,7 +16,7 @@ func MigrateTombstoneColumns(db *sql.DB) error { name string definition string }{ - {"deleted_at", "TEXT"}, + {"deleted_at", "DATETIME"}, {"deleted_by", "TEXT DEFAULT ''"}, {"delete_reason", "TEXT DEFAULT ''"}, {"original_type", "TEXT DEFAULT ''"}, diff --git a/internal/storage/sqlite/queries.go b/internal/storage/sqlite/queries.go index 76ad6932..e824a20d 100644 --- a/internal/storage/sqlite/queries.go +++ b/internal/storage/sqlite/queries.go @@ -836,6 +836,76 @@ func (s *SQLiteStorage) CloseIssue(ctx context.Context, id string, reason string return tx.Commit() } +// CreateTombstone converts an existing issue to a tombstone record. +// This is a soft-delete that preserves the issue in the database with status="tombstone". +// The issue will still appear in exports but be excluded from normal queries. +// Dependencies must be removed separately before calling this method. +func (s *SQLiteStorage) CreateTombstone(ctx context.Context, id string, actor string, reason string) error { + // Get the issue to preserve its original type + issue, err := s.GetIssue(ctx, id) + if err != nil { + return fmt.Errorf("failed to get issue: %w", err) + } + if issue == nil { + return fmt.Errorf("issue not found: %s", id) + } + + tx, err := s.db.BeginTx(ctx, nil) + if err != nil { + return fmt.Errorf("failed to begin transaction: %w", err) + } + defer func() { _ = tx.Rollback() }() + + now := time.Now() + originalType := string(issue.IssueType) + + // Convert issue to tombstone + _, err = tx.ExecContext(ctx, ` + UPDATE issues + SET status = ?, + deleted_at = ?, + deleted_by = ?, + delete_reason = ?, + original_type = ?, + updated_at = ? + WHERE id = ? + `, types.StatusTombstone, now, actor, reason, originalType, now, id) + if err != nil { + return fmt.Errorf("failed to create tombstone: %w", err) + } + + // Record tombstone creation event + _, err = tx.ExecContext(ctx, ` + INSERT INTO events (issue_id, event_type, actor, comment) + VALUES (?, ?, ?, ?) + `, id, "deleted", actor, reason) + if err != nil { + return fmt.Errorf("failed to record tombstone event: %w", err) + } + + // Mark issue as dirty for incremental export + _, err = tx.ExecContext(ctx, ` + INSERT INTO dirty_issues (issue_id, marked_at) + VALUES (?, ?) + ON CONFLICT (issue_id) DO UPDATE SET marked_at = excluded.marked_at + `, id, now) + if err != nil { + return fmt.Errorf("failed to mark issue dirty: %w", err) + } + + // Invalidate blocked issues cache since status changed (bd-5qim) + // Tombstone issues don't block others, so this affects blocking calculations + if err := s.invalidateBlockedCache(ctx, tx); err != nil { + return fmt.Errorf("failed to invalidate blocked cache: %w", err) + } + + if err := tx.Commit(); err != nil { + return wrapDBError("commit tombstone transaction", err) + } + + return nil +} + // DeleteIssue permanently removes an issue from the database func (s *SQLiteStorage) DeleteIssue(ctx context.Context, id string) error { tx, err := s.db.BeginTx(ctx, nil) @@ -1086,30 +1156,85 @@ func (s *SQLiteStorage) populateDeleteStats(ctx context.Context, tx *sql.Tx, inC } func (s *SQLiteStorage) executeDelete(ctx context.Context, tx *sql.Tx, inClause string, args []interface{}, result *DeleteIssuesResult) error { - deletes := []struct { - query string - args []interface{} - }{ - {fmt.Sprintf(`DELETE FROM dependencies WHERE issue_id IN (%s) OR depends_on_id IN (%s)`, inClause, inClause), append(args, args...)}, - {fmt.Sprintf(`DELETE FROM labels WHERE issue_id IN (%s)`, inClause), args}, - {fmt.Sprintf(`DELETE FROM events WHERE issue_id IN (%s)`, inClause), args}, - {fmt.Sprintf(`DELETE FROM dirty_issues WHERE issue_id IN (%s)`, inClause), args}, - {fmt.Sprintf(`DELETE FROM issues WHERE id IN (%s)`, inClause), args}, + // Note: This method now creates tombstones instead of hard-deleting (bd-3b4) + // Only dependencies are deleted - issues are converted to tombstones + + // 1. Delete dependencies - tombstones don't block other issues + _, err := tx.ExecContext(ctx, + fmt.Sprintf(`DELETE FROM dependencies WHERE issue_id IN (%s) OR depends_on_id IN (%s)`, inClause, inClause), + append(args, args...)...) + if err != nil { + return fmt.Errorf("failed to delete dependencies: %w", err) } - for i, d := range deletes { - execResult, err := tx.ExecContext(ctx, d.query, d.args...) - if err != nil { - return fmt.Errorf("failed to delete: %w", err) + // 2. Get issue types before converting to tombstones (need for original_type) + issueTypes := make(map[string]string) + rows, err := tx.QueryContext(ctx, + fmt.Sprintf(`SELECT id, issue_type FROM issues WHERE id IN (%s)`, inClause), + args...) + if err != nil { + return fmt.Errorf("failed to get issue types: %w", err) + } + for rows.Next() { + var id, issueType string + if err := rows.Scan(&id, &issueType); err != nil { + rows.Close() + return fmt.Errorf("failed to scan issue type: %w", err) } - if i == len(deletes)-1 { - rowsAffected, err := execResult.RowsAffected() - if err != nil { - return fmt.Errorf("failed to check rows affected: %w", err) - } - result.DeletedCount = int(rowsAffected) + issueTypes[id] = issueType + } + rows.Close() + + // 3. Convert issues to tombstones (only for issues that exist) + now := time.Now() + deletedCount := 0 + for id, originalType := range issueTypes { + execResult, err := tx.ExecContext(ctx, ` + UPDATE issues + SET status = ?, + deleted_at = ?, + deleted_by = ?, + delete_reason = ?, + original_type = ?, + updated_at = ? + WHERE id = ? + `, types.StatusTombstone, now, "batch delete", "batch delete", originalType, now, id) + if err != nil { + return fmt.Errorf("failed to create tombstone for %s: %w", id, err) + } + + rowsAffected, _ := execResult.RowsAffected() + if rowsAffected == 0 { + continue // Issue doesn't exist, skip + } + deletedCount++ + + // Record tombstone creation event + _, err = tx.ExecContext(ctx, ` + INSERT INTO events (issue_id, event_type, actor, comment) + VALUES (?, ?, ?, ?) + `, id, "deleted", "batch delete", "batch delete") + if err != nil { + return fmt.Errorf("failed to record tombstone event for %s: %w", id, err) + } + + // Mark issue as dirty for incremental export + _, err = tx.ExecContext(ctx, ` + INSERT INTO dirty_issues (issue_id, marked_at) + VALUES (?, ?) + ON CONFLICT (issue_id) DO UPDATE SET marked_at = excluded.marked_at + `, id, now) + if err != nil { + return fmt.Errorf("failed to mark issue dirty for %s: %w", id, err) } } + + // 4. Invalidate blocked issues cache since statuses changed (bd-5qim) + if err := s.invalidateBlockedCache(ctx, tx); err != nil { + return fmt.Errorf("failed to invalidate blocked cache: %w", err) + } + + result.DeletedCount = deletedCount return nil } diff --git a/internal/storage/sqlite/schema.go b/internal/storage/sqlite/schema.go index baa65c1e..286feef1 100644 --- a/internal/storage/sqlite/schema.go +++ b/internal/storage/sqlite/schema.go @@ -23,7 +23,7 @@ CREATE TABLE IF NOT EXISTS issues ( compacted_at DATETIME, compacted_at_commit TEXT, original_size INTEGER, - deleted_at TEXT, + deleted_at DATETIME, deleted_by TEXT DEFAULT '', delete_reason TEXT DEFAULT '', original_type TEXT DEFAULT '', diff --git a/internal/storage/sqlite/tombstone_test.go b/internal/storage/sqlite/tombstone_test.go new file mode 100644 index 00000000..1b48c18c --- /dev/null +++ b/internal/storage/sqlite/tombstone_test.go @@ -0,0 +1,332 @@ +package sqlite + +import ( + "context" + "testing" + + "github.com/steveyegge/beads/internal/types" +) + +func TestCreateTombstone(t *testing.T) { + store := newTestStore(t, "file::memory:?mode=memory&cache=private") + ctx := context.Background() + + t.Run("create tombstone for existing issue", func(t *testing.T) { + issue := &types.Issue{ + ID: "bd-1", + Title: "Test Issue", + Status: types.StatusOpen, + Priority: 1, + IssueType: types.TypeTask, + } + + if err := store.CreateIssue(ctx, issue, "test"); err != nil { + t.Fatalf("Failed to create issue: %v", err) + } + + // Create tombstone + if err := store.CreateTombstone(ctx, "bd-1", "tester", "testing tombstone"); err != nil { + t.Fatalf("CreateTombstone failed: %v", err) + } + + // Verify issue still exists but is a tombstone + tombstone, err := store.GetIssue(ctx, "bd-1") + if err != nil { + t.Fatalf("Failed to get tombstone: %v", err) + } + if tombstone == nil { + t.Fatal("Tombstone should still exist in database") + } + if tombstone.Status != types.StatusTombstone { + t.Errorf("Expected status=tombstone, got %s", tombstone.Status) + } + if tombstone.DeletedAt == nil { + t.Error("DeletedAt should be set") + } + if tombstone.DeletedBy != "tester" { + t.Errorf("Expected DeletedBy=tester, got %s", tombstone.DeletedBy) + } + if tombstone.DeleteReason != "testing tombstone" { + t.Errorf("Expected DeleteReason='testing tombstone', got %s", tombstone.DeleteReason) + } + if tombstone.OriginalType != string(types.TypeTask) { + t.Errorf("Expected OriginalType=task, got %s", tombstone.OriginalType) + } + }) + + t.Run("create tombstone for non-existent issue", func(t *testing.T) { + err := store.CreateTombstone(ctx, "bd-999", "tester", "testing") + if err == nil { + t.Error("CreateTombstone should fail for non-existent issue") + } + }) + + t.Run("tombstone excluded from normal search", func(t *testing.T) { + // Create two issues + issue1 := &types.Issue{ + ID: "bd-10", + Title: "Active Issue", + Status: types.StatusOpen, + Priority: 1, + IssueType: types.TypeTask, + } + issue2 := &types.Issue{ + ID: "bd-11", + Title: "To Be Deleted", + Status: types.StatusOpen, + Priority: 1, + IssueType: types.TypeTask, + } + + if err := store.CreateIssue(ctx, issue1, "test"); err != nil { + t.Fatalf("Failed to create issue1: %v", err) + } + if err := store.CreateIssue(ctx, issue2, "test"); err != nil { + t.Fatalf("Failed to create issue2: %v", err) + } + + // Create tombstone for issue2 + if err := store.CreateTombstone(ctx, "bd-11", "tester", "test"); err != nil { + t.Fatalf("CreateTombstone failed: %v", err) + } + + // Search without tombstones + results, err := store.SearchIssues(ctx, "", types.IssueFilter{}) + if err != nil { + t.Fatalf("SearchIssues failed: %v", err) + } + + // Should only return bd-10 (active issue) + foundActive := false + foundTombstone := false + for _, issue := range results { + if issue.ID == "bd-10" { + foundActive = true + } + if issue.ID == "bd-11" { + foundTombstone = true + } + } + + if !foundActive { + t.Error("Active issue bd-10 should be in results") + } + if foundTombstone { + t.Error("Tombstone bd-11 should not be in results") + } + }) + + t.Run("tombstone included with IncludeTombstones flag", func(t *testing.T) { + // Search with tombstones included + filter := types.IssueFilter{IncludeTombstones: true} + results, err := store.SearchIssues(ctx, "", filter) + if err != nil { + t.Fatalf("SearchIssues failed: %v", err) + } + + // Should return both active and tombstone + foundActive := false + foundTombstone := false + for _, issue := range results { + if issue.ID == "bd-10" { + foundActive = true + } + if issue.ID == "bd-11" { + foundTombstone = true + } + } + + if !foundActive { + t.Error("Active issue bd-10 should be in results") + } + if !foundTombstone { + t.Error("Tombstone bd-11 should be in results when IncludeTombstones=true") + } + }) + + t.Run("search for tombstones explicitly", func(t *testing.T) { + // Search for tombstone status explicitly + status := types.StatusTombstone + filter := types.IssueFilter{Status: &status} + results, err := store.SearchIssues(ctx, "", filter) + if err != nil { + t.Fatalf("SearchIssues failed: %v", err) + } + + // Should only return tombstones + for _, issue := range results { + if issue.Status != types.StatusTombstone { + t.Errorf("Expected only tombstones, found %s with status %s", issue.ID, issue.Status) + } + } + + // Should find at least bd-11 + foundTombstone := false + for _, issue := range results { + if issue.ID == "bd-11" { + foundTombstone = true + } + } + if !foundTombstone { + t.Error("Should find tombstone bd-11") + } + }) +} + +func TestDeleteIssuesCreatesTombstones(t *testing.T) { + ctx := context.Background() + + t.Run("single issue deletion creates tombstone", func(t *testing.T) { + store := newTestStore(t, "file::memory:?mode=memory&cache=private") + + issue := &types.Issue{ID: "bd-1", Title: "Test", Status: types.StatusOpen, Priority: 1, IssueType: types.TypeFeature} + if err := store.CreateIssue(ctx, issue, "test"); err != nil { + t.Fatalf("Failed to create issue: %v", err) + } + + result, err := store.DeleteIssues(ctx, []string{"bd-1"}, false, true, false) + if err != nil { + t.Fatalf("DeleteIssues failed: %v", err) + } + if result.DeletedCount != 1 { + t.Errorf("Expected 1 deletion, got %d", result.DeletedCount) + } + + // Issue should still exist as tombstone + tombstone, err := store.GetIssue(ctx, "bd-1") + if err != nil { + t.Fatalf("Failed to get issue: %v", err) + } + if tombstone == nil { + t.Fatal("Issue should exist as tombstone") + } + if tombstone.Status != types.StatusTombstone { + t.Errorf("Expected tombstone status, got %s", tombstone.Status) + } + if tombstone.OriginalType != string(types.TypeFeature) { + t.Errorf("Expected OriginalType=feature, got %s", tombstone.OriginalType) + } + }) + + t.Run("batch deletion creates tombstones", func(t *testing.T) { + store := newTestStore(t, "file::memory:?mode=memory&cache=private") + + issue1 := &types.Issue{ID: "bd-10", Title: "Test 1", Status: types.StatusOpen, Priority: 1, IssueType: types.TypeBug} + issue2 := &types.Issue{ID: "bd-11", Title: "Test 2", Status: types.StatusOpen, Priority: 1, IssueType: types.TypeTask} + + if err := store.CreateIssue(ctx, issue1, "test"); err != nil { + t.Fatalf("Failed to create issue1: %v", err) + } + if err := store.CreateIssue(ctx, issue2, "test"); err != nil { + t.Fatalf("Failed to create issue2: %v", err) + } + + result, err := store.DeleteIssues(ctx, []string{"bd-10", "bd-11"}, false, true, false) + if err != nil { + t.Fatalf("DeleteIssues failed: %v", err) + } + if result.DeletedCount != 2 { + t.Errorf("Expected 2 deletions, got %d", result.DeletedCount) + } + + // Both should exist as tombstones + tombstone1, _ := store.GetIssue(ctx, "bd-10") + if tombstone1 == nil || tombstone1.Status != types.StatusTombstone { + t.Error("bd-10 should be tombstone") + } + if tombstone1.OriginalType != string(types.TypeBug) { + t.Errorf("bd-10: Expected OriginalType=bug, got %s", tombstone1.OriginalType) + } + + tombstone2, _ := store.GetIssue(ctx, "bd-11") + if tombstone2 == nil || tombstone2.Status != types.StatusTombstone { + t.Error("bd-11 should be tombstone") + } + if tombstone2.OriginalType != string(types.TypeTask) { + t.Errorf("bd-11: Expected OriginalType=task, got %s", tombstone2.OriginalType) + } + }) + + t.Run("cascade deletion creates tombstones", func(t *testing.T) { + store := newTestStore(t, "file::memory:?mode=memory&cache=private") + + // Create chain: bd-1 -> bd-2 -> bd-3 + issue1 := &types.Issue{ID: "bd-1", Title: "Parent", Status: types.StatusOpen, Priority: 1, IssueType: types.TypeEpic} + issue2 := &types.Issue{ID: "bd-2", Title: "Child", Status: types.StatusOpen, Priority: 1, IssueType: types.TypeTask} + issue3 := &types.Issue{ID: "bd-3", Title: "Grandchild", Status: types.StatusOpen, Priority: 1, IssueType: types.TypeTask} + + if err := store.CreateIssue(ctx, issue1, "test"); err != nil { + t.Fatalf("Failed to create issue1: %v", err) + } + if err := store.CreateIssue(ctx, issue2, "test"); err != nil { + t.Fatalf("Failed to create issue2: %v", err) + } + if err := store.CreateIssue(ctx, issue3, "test"); err != nil { + t.Fatalf("Failed to create issue3: %v", err) + } + + dep1 := &types.Dependency{IssueID: "bd-2", DependsOnID: "bd-1", Type: types.DepBlocks} + if err := store.AddDependency(ctx, dep1, "test"); err != nil { + t.Fatalf("Failed to add dependency: %v", err) + } + dep2 := &types.Dependency{IssueID: "bd-3", DependsOnID: "bd-2", Type: types.DepBlocks} + if err := store.AddDependency(ctx, dep2, "test"); err != nil { + t.Fatalf("Failed to add dependency: %v", err) + } + + result, err := store.DeleteIssues(ctx, []string{"bd-1"}, true, false, false) + if err != nil { + t.Fatalf("DeleteIssues with cascade failed: %v", err) + } + if result.DeletedCount != 3 { + t.Errorf("Expected 3 deletions (cascade), got %d", result.DeletedCount) + } + + // All should exist as tombstones + for _, id := range []string{"bd-1", "bd-2", "bd-3"} { + tombstone, _ := store.GetIssue(ctx, id) + if tombstone == nil { + t.Errorf("%s should exist as tombstone", id) + continue + } + if tombstone.Status != types.StatusTombstone { + t.Errorf("%s should have tombstone status, got %s", id, tombstone.Status) + } + } + }) + + t.Run("dependencies removed from tombstones", func(t *testing.T) { + store := newTestStore(t, "file::memory:?mode=memory&cache=private") + + // Create issues with dependency + issue1 := &types.Issue{ID: "bd-100", Title: "Parent", Status: types.StatusOpen, Priority: 1, IssueType: types.TypeTask} + issue2 := &types.Issue{ID: "bd-101", Title: "Child", Status: types.StatusOpen, Priority: 1, IssueType: types.TypeTask} + + if err := store.CreateIssue(ctx, issue1, "test"); err != nil { + t.Fatalf("Failed to create issue1: %v", err) + } + if err := store.CreateIssue(ctx, issue2, "test"); err != nil { + t.Fatalf("Failed to create issue2: %v", err) + } + + dep := &types.Dependency{IssueID: "bd-101", DependsOnID: "bd-100", Type: types.DepBlocks} + if err := store.AddDependency(ctx, dep, "test"); err != nil { + t.Fatalf("Failed to add dependency: %v", err) + } + + // Delete parent + _, err := store.DeleteIssues(ctx, []string{"bd-100"}, false, true, false) + if err != nil { + t.Fatalf("DeleteIssues failed: %v", err) + } + + // Dependency should be removed + deps, err := store.GetDependencies(ctx, "bd-101") + if err != nil { + t.Fatalf("GetDependencies failed: %v", err) + } + if len(deps) != 0 { + t.Errorf("Dependency should be removed, found %d dependencies", len(deps)) + } + }) +} diff --git a/internal/types/types.go b/internal/types/types.go index e2e045a5..7f0475a4 100644 --- a/internal/types/types.go +++ b/internal/types/types.go @@ -74,11 +74,47 @@ func (i *Issue) ComputeContentHash() string { return fmt.Sprintf("%x", h.Sum(nil)) } +// DefaultTombstoneTTL is the default time-to-live for tombstones (30 days) +const DefaultTombstoneTTL = 30 * 24 * time.Hour + +// MinTombstoneTTL is the minimum allowed TTL (7 days) to prevent data loss +const MinTombstoneTTL = 7 * 24 * time.Hour + +// ClockSkewGrace is added to TTL to handle clock drift between machines +const ClockSkewGrace = 1 * time.Hour + // IsTombstone returns true if the issue has been soft-deleted (bd-vw8) func (i *Issue) IsTombstone() bool { return i.Status == StatusTombstone } +// IsExpired returns true if the tombstone has exceeded its TTL. +// Non-tombstone issues always return false. +// ttl is the configured TTL duration; if zero, DefaultTombstoneTTL is used. +func (i *Issue) IsExpired(ttl time.Duration) bool { + // Non-tombstones never expire + if !i.IsTombstone() { + return false + } + + // Tombstones without DeletedAt are not expired (safety: shouldn't happen in valid data) + if i.DeletedAt == nil { + return false + } + + // Use default TTL if not specified + if ttl == 0 { + ttl = DefaultTombstoneTTL + } + + // Add clock skew grace period to the TTL + effectiveTTL := ttl + ClockSkewGrace + + // Check if the tombstone has exceeded its TTL + expirationTime := i.DeletedAt.Add(effectiveTTL) + return time.Now().After(expirationTime) +} + // Validate checks if the issue has valid field values (built-in statuses only) func (i *Issue) Validate() error { return i.ValidateWithCustomStatuses(nil) diff --git a/internal/types/types_test.go b/internal/types/types_test.go index 79650759..25fd647a 100644 --- a/internal/types/types_test.go +++ b/internal/types/types_test.go @@ -579,6 +579,213 @@ func TestSortPolicyIsValid(t *testing.T) { } } +func TestIsExpired(t *testing.T) { + now := time.Now() + + tests := []struct { + name string + issue Issue + ttl time.Duration + expired bool + }{ + { + name: "non-tombstone issue never expires", + issue: Issue{ + ID: "test-1", + Title: "Open issue", + Status: StatusOpen, + Priority: 2, + IssueType: TypeTask, + }, + ttl: 0, + expired: false, + }, + { + name: "closed issue never expires", + issue: Issue{ + ID: "test-2", + Title: "Closed issue", + Status: StatusClosed, + Priority: 2, + IssueType: TypeTask, + ClosedAt: timePtr(now), + }, + ttl: 0, + expired: false, + }, + { + name: "tombstone without DeletedAt does not expire", + issue: Issue{ + ID: "test-3", + Title: "(deleted)", + Status: StatusTombstone, + Priority: 0, + IssueType: TypeTask, + DeletedAt: nil, + }, + ttl: 0, + expired: false, + }, + { + name: "tombstone within default TTL (30 days)", + issue: Issue{ + ID: "test-4", + Title: "(deleted)", + Status: StatusTombstone, + Priority: 0, + IssueType: TypeTask, + DeletedAt: timePtr(now.Add(-15 * 24 * time.Hour)), // 15 days ago + }, + ttl: 0, // Use default TTL + expired: false, + }, + { + name: "tombstone past default TTL (30 days)", + issue: Issue{ + ID: "test-5", + Title: "(deleted)", + Status: StatusTombstone, + Priority: 0, + IssueType: TypeTask, + DeletedAt: timePtr(now.Add(-35 * 24 * time.Hour)), // 35 days ago (past 30 days + 1 hour grace) + }, + ttl: 0, // Use default TTL + expired: true, + }, + { + name: "tombstone within custom TTL (7 days)", + issue: Issue{ + ID: "test-6", + Title: "(deleted)", + Status: StatusTombstone, + Priority: 0, + IssueType: TypeTask, + DeletedAt: timePtr(now.Add(-3 * 24 * time.Hour)), // 3 days ago + }, + ttl: 7 * 24 * time.Hour, + expired: false, + }, + { + name: "tombstone past custom TTL (7 days)", + issue: Issue{ + ID: "test-7", + Title: "(deleted)", + Status: StatusTombstone, + Priority: 0, + IssueType: TypeTask, + DeletedAt: timePtr(now.Add(-9 * 24 * time.Hour)), // 9 days ago (past 7 days + 1 hour grace) + }, + ttl: 7 * 24 * time.Hour, + expired: true, + }, + { + name: "tombstone at exact TTL boundary (within grace period)", + issue: Issue{ + ID: "test-8", + Title: "(deleted)", + Status: StatusTombstone, + Priority: 0, + IssueType: TypeTask, + DeletedAt: timePtr(now.Add(-30 * 24 * time.Hour)), // Exactly 30 days ago + }, + ttl: 0, // Use default TTL (30 days + 1 hour grace) + expired: false, + }, + { + name: "tombstone just past TTL boundary (beyond grace period)", + issue: Issue{ + ID: "test-9", + Title: "(deleted)", + Status: StatusTombstone, + Priority: 0, + IssueType: TypeTask, + DeletedAt: timePtr(now.Add(-(30*24*time.Hour + 2*time.Hour))), // 30 days + 2 hours ago + }, + ttl: 0, // Use default TTL (30 days + 1 hour grace) + expired: true, + }, + { + name: "tombstone within grace period", + issue: Issue{ + ID: "test-10", + Title: "(deleted)", + Status: StatusTombstone, + Priority: 0, + IssueType: TypeTask, + DeletedAt: timePtr(now.Add(-(30*24*time.Hour + 30*time.Minute))), // 30 days + 30 minutes ago + }, + ttl: 0, // Use default TTL (30 days + 1 hour grace) + expired: false, + }, + { + name: "tombstone with MinTombstoneTTL (7 days)", + issue: Issue{ + ID: "test-11", + Title: "(deleted)", + Status: StatusTombstone, + Priority: 0, + IssueType: TypeTask, + DeletedAt: timePtr(now.Add(-10 * 24 * time.Hour)), // 10 days ago + }, + ttl: MinTombstoneTTL, // 7 days + expired: true, + }, + { + name: "tombstone with very short TTL (1 hour)", + issue: Issue{ + ID: "test-12", + Title: "(deleted)", + Status: StatusTombstone, + Priority: 0, + IssueType: TypeTask, + DeletedAt: timePtr(now.Add(-3 * time.Hour)), // 3 hours ago + }, + ttl: 1 * time.Hour, // 1 hour + 1 hour grace = 2 hours total + expired: true, + }, + { + name: "tombstone deleted in the future (clock skew)", + issue: Issue{ + ID: "test-13", + Title: "(deleted)", + Status: StatusTombstone, + Priority: 0, + IssueType: TypeTask, + DeletedAt: timePtr(now.Add(1 * time.Hour)), // 1 hour in the future + }, + ttl: 7 * 24 * time.Hour, + expired: false, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + got := tt.issue.IsExpired(tt.ttl) + if got != tt.expired { + t.Errorf("Issue.IsExpired(%v) = %v, want %v", tt.ttl, got, tt.expired) + } + }) + } +} + +func TestTombstoneTTLConstants(t *testing.T) { + // Test that constants have expected values + if DefaultTombstoneTTL != 30*24*time.Hour { + t.Errorf("DefaultTombstoneTTL = %v, want %v", DefaultTombstoneTTL, 30*24*time.Hour) + } + if MinTombstoneTTL != 7*24*time.Hour { + t.Errorf("MinTombstoneTTL = %v, want %v", MinTombstoneTTL, 7*24*time.Hour) + } + if ClockSkewGrace != 1*time.Hour { + t.Errorf("ClockSkewGrace = %v, want %v", ClockSkewGrace, 1*time.Hour) + } + + // Test that MinTombstoneTTL is less than DefaultTombstoneTTL + if MinTombstoneTTL >= DefaultTombstoneTTL { + t.Errorf("MinTombstoneTTL (%v) should be less than DefaultTombstoneTTL (%v)", MinTombstoneTTL, DefaultTombstoneTTL) + } +} + // Helper functions func intPtr(i int) *int {