From 0ce039429dbf8d646915aa8241ee427d9f5228ad Mon Sep 17 00:00:00 2001 From: Steve Yegge Date: Tue, 16 Dec 2025 22:01:31 -0800 Subject: [PATCH] fix: tombstone/deletion overhaul for bd-4q8 MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - IsExpired(): Negative TTL means immediately expired (for --hard mode) - IsExpired(): ClockSkewGrace only added for TTLs > 1 hour - bd cleanup --hard: Use negative TTL to prune freshly created tombstones - bd delete --hard: New flag to immediately prune tombstones from JSONL - Import: Add early tombstone check before all phases to prevent resurrection The early tombstone check prevents ghost issues from being created when tombstones exist in the DB. However, a deeper git merge issue (bd-ncwo) can still cause resurrection when remote's status:closed wins the merge. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Opus 4.5 --- cmd/bd/cleanup.go | 8 ++++--- cmd/bd/delete.go | 44 +++++++++++++++++++++++++++++++---- internal/importer/importer.go | 12 +++++++++- internal/types/types.go | 18 +++++++++++--- internal/types/types_test.go | 25 ++++++++++++++++++++ 5 files changed, 95 insertions(+), 12 deletions(-) diff --git a/cmd/bd/cleanup.go b/cmd/bd/cleanup.go index 00577443..5448147c 100644 --- a/cmd/bd/cleanup.go +++ b/cmd/bd/cleanup.go @@ -77,8 +77,9 @@ SEE ALSO: if olderThanDays > 0 { customTTL = time.Duration(olderThanDays) * 24 * time.Hour } else { - // --hard without --older-than: prune ALL tombstones (use 1 second TTL) - customTTL = time.Second + // --hard without --older-than: prune ALL tombstones immediately + // Negative TTL means "immediately expired" (bd-4q8 fix) + customTTL = -1 } if !jsonOutput && !dryRun { fmt.Println(color.YellowString("⚠️ HARD DELETE MODE: Bypassing tombstone TTL safety")) @@ -186,7 +187,8 @@ SEE ALSO: } // Use the existing batch deletion logic - deleteBatch(cmd, issueIDs, force, dryRun, cascade, jsonOutput, "cleanup") + // Note: cleanup always creates tombstones first; --hard prunes them after + deleteBatch(cmd, issueIDs, force, dryRun, cascade, jsonOutput, false, "cleanup") // Also prune expired tombstones (bd-08ea) // This runs after closed issues are converted to tombstones, cleaning up old ones diff --git a/cmd/bd/delete.go b/cmd/bd/delete.go index 9228a3fc..3435390d 100644 --- a/cmd/bd/delete.go +++ b/cmd/bd/delete.go @@ -91,22 +91,36 @@ var deleteCmd = &cobra.Command{ This command will: 1. Remove all dependency links (any type, both directions) involving the issues 2. Update text references to "[deleted:ID]" in directly connected issues -3. Delete the issues from the database +3. Delete the issues from the database (creates tombstones by default) + This is a destructive operation that cannot be undone. Use with caution. + BATCH DELETION: Delete multiple issues at once: bd delete bd-1 bd-2 bd-3 --force + Delete from file (one ID per line): bd delete --from-file deletions.txt --force + Preview before deleting: bd delete --from-file deletions.txt --dry-run + DEPENDENCY HANDLING: Default: Fails if any issue has dependents not in deletion set bd delete bd-1 bd-2 + Cascade: Recursively delete all dependents bd delete bd-1 --cascade --force + Force: Delete and orphan dependents - bd delete bd-1 --force`, + bd delete bd-1 --force + +PERMANENT DELETION: +Use --hard to permanently delete (bypass tombstones): + bd delete bd-1 bd-2 --hard --force + +WARNING: --hard bypasses sync safety. Use only when you are certain +the issues will not resurrect from remote branches.`, Args: cobra.MinimumNArgs(0), Run: func(cmd *cobra.Command, args []string) { CheckReadonly("delete") @@ -114,6 +128,7 @@ Force: Delete and orphan dependents force, _ := cmd.Flags().GetBool("force") dryRun, _ := cmd.Flags().GetBool("dry-run") cascade, _ := cmd.Flags().GetBool("cascade") + hardDelete, _ := cmd.Flags().GetBool("hard") // Use global jsonOutput set by PersistentPreRun // Collect issue IDs from args and/or file issueIDs := make([]string, 0, len(args)) @@ -150,7 +165,7 @@ Force: Delete and orphan dependents // Handle batch deletion in direct mode if len(issueIDs) > 1 { - deleteBatch(cmd, issueIDs, force, dryRun, cascade, jsonOutput, "batch delete") + deleteBatch(cmd, issueIDs, force, dryRun, cascade, jsonOutput, hardDelete, "batch delete") return } @@ -409,7 +424,7 @@ func removeIssueFromJSONL(issueID string) error { } // deleteBatch handles deletion of multiple issues //nolint:unparam // cmd parameter required for potential future use -func deleteBatch(_ *cobra.Command, issueIDs []string, force bool, dryRun bool, cascade bool, jsonOutput bool, reason string) { +func deleteBatch(_ *cobra.Command, issueIDs []string, force bool, dryRun bool, cascade bool, jsonOutput bool, hardDelete bool, reason string) { // Ensure we have a direct store if store == nil { if err := ensureStoreActive(); err != nil { @@ -499,12 +514,30 @@ func deleteBatch(_ *cobra.Command, issueIDs []string, force bool, dryRun bool, c } } } - // Actually delete + // Actually delete (creates tombstones) result, err := d.DeleteIssues(ctx, issueIDs, cascade, force, false) if err != nil { fmt.Fprintf(os.Stderr, "Error: %v\n", err) os.Exit(1) } + + // Hard delete: immediately prune tombstones from JSONL (bd-4q8) + // Note: We keep tombstones in DB to prevent resurrection during sync. + // The tombstones will be exported and synced to remote, blocking resurrection. + // Use 'bd cleanup --hard' after syncing to fully purge old tombstones. + if hardDelete { + if !jsonOutput { + fmt.Println(color.YellowString("⚠️ HARD DELETE MODE: Pruning tombstones from JSONL")) + fmt.Println(" Note: Tombstones kept in DB to prevent resurrection. Run 'bd sync' then 'bd cleanup --hard' to fully purge.") + } + // Prune tombstones from JSONL using negative TTL (immediate expiration) + if pruneResult, err := pruneExpiredTombstones(-1); err != nil { + fmt.Fprintf(os.Stderr, "Warning: failed to prune tombstones from JSONL: %v\n", err) + } else if pruneResult != nil && pruneResult.PrunedCount > 0 && !jsonOutput { + fmt.Printf(" Pruned %d tombstone(s) from JSONL\n", pruneResult.PrunedCount) + } + } + // Update text references in connected issues (using pre-collected issues) updatedCount := updateTextReferencesInIssues(ctx, issueIDs, connectedIssues) // Note: No longer remove from JSONL - tombstones will be exported to JSONL (bd-3b4) @@ -663,5 +696,6 @@ func init() { deleteCmd.Flags().String("from-file", "", "Read issue IDs from file (one per line)") deleteCmd.Flags().Bool("dry-run", false, "Preview what would be deleted without making changes") deleteCmd.Flags().Bool("cascade", false, "Recursively delete all dependent issues") + deleteCmd.Flags().Bool("hard", false, "Permanently delete (skip tombstone, cannot be recovered via sync)") rootCmd.AddCommand(deleteCmd) } diff --git a/internal/importer/importer.go b/internal/importer/importer.go index 52cda778..262bf62c 100644 --- a/internal/importer/importer.go +++ b/internal/importer/importer.go @@ -492,7 +492,17 @@ func upsertIssues(ctx context.Context, sqliteStore *sqlite.SQLiteStorage, issues continue } seenHashes[hash] = true - + + // CRITICAL: Check for tombstone FIRST, before any other matching (bd-4q8 fix) + // This prevents ghost resurrection regardless of which phase would normally match. + // If this ID has a tombstone in the DB, skip importing it entirely. + if existingByID, found := dbByID[incoming.ID]; found { + if existingByID.Status == types.StatusTombstone { + result.Skipped++ + continue + } + } + // Phase 0: Match by external_ref first (if present) // This enables re-syncing from external systems (Jira, GitHub, Linear) if incoming.ExternalRef != nil && *incoming.ExternalRef != "" { diff --git a/internal/types/types.go b/internal/types/types.go index 86afe14d..3915e940 100644 --- a/internal/types/types.go +++ b/internal/types/types.go @@ -98,7 +98,10 @@ func (i *Issue) IsTombstone() bool { // 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. +// ttl is the configured TTL duration: +// - If zero, DefaultTombstoneTTL (30 days) is used +// - If negative, the tombstone is immediately expired (for --hard mode) +// - If positive, ClockSkewGrace is added only for TTLs > 1 hour func (i *Issue) IsExpired(ttl time.Duration) bool { // Non-tombstones never expire if !i.IsTombstone() { @@ -110,13 +113,22 @@ func (i *Issue) IsExpired(ttl time.Duration) bool { return false } + // Negative TTL means "immediately expired" - for --hard mode (bd-4q8 fix) + if ttl < 0 { + return true + } + // Use default TTL if not specified if ttl == 0 { ttl = DefaultTombstoneTTL } - // Add clock skew grace period to the TTL - effectiveTTL := ttl + ClockSkewGrace + // Only add clock skew grace period for normal TTLs (> 1 hour). + // For short TTLs (testing/development), skip grace period. + effectiveTTL := ttl + if ttl > ClockSkewGrace { + effectiveTTL = ttl + ClockSkewGrace + } // Check if the tombstone has exceeded its TTL expirationTime := i.DeletedAt.Add(effectiveTTL) diff --git a/internal/types/types_test.go b/internal/types/types_test.go index 25fd647a..904e7045 100644 --- a/internal/types/types_test.go +++ b/internal/types/types_test.go @@ -756,6 +756,31 @@ func TestIsExpired(t *testing.T) { ttl: 7 * 24 * time.Hour, expired: false, }, + { + name: "negative TTL means immediately expired (bd-4q8 --hard mode)", + issue: Issue{ + ID: "test-14", + Title: "(deleted)", + Status: StatusTombstone, + Priority: 0, + IssueType: TypeTask, + DeletedAt: timePtr(now), // Just deleted NOW + }, + ttl: -1, // Negative TTL = immediate expiration + expired: true, + }, + { + name: "non-tombstone never expires even with negative TTL", + issue: Issue{ + ID: "test-15", + Title: "Open issue", + Status: StatusOpen, + Priority: 0, + IssueType: TypeTask, + }, + ttl: -1, + expired: false, + }, } for _, tt := range tests {