fix: tombstone/deletion overhaul for bd-4q8

- 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 <noreply@anthropic.com>
This commit is contained in:
Steve Yegge
2025-12-16 22:01:31 -08:00
parent 8d73a86f7a
commit 0ce039429d
5 changed files with 95 additions and 12 deletions

View File

@@ -77,8 +77,9 @@ SEE ALSO:
if olderThanDays > 0 { if olderThanDays > 0 {
customTTL = time.Duration(olderThanDays) * 24 * time.Hour customTTL = time.Duration(olderThanDays) * 24 * time.Hour
} else { } else {
// --hard without --older-than: prune ALL tombstones (use 1 second TTL) // --hard without --older-than: prune ALL tombstones immediately
customTTL = time.Second // Negative TTL means "immediately expired" (bd-4q8 fix)
customTTL = -1
} }
if !jsonOutput && !dryRun { if !jsonOutput && !dryRun {
fmt.Println(color.YellowString("⚠️ HARD DELETE MODE: Bypassing tombstone TTL safety")) fmt.Println(color.YellowString("⚠️ HARD DELETE MODE: Bypassing tombstone TTL safety"))
@@ -186,7 +187,8 @@ SEE ALSO:
} }
// Use the existing batch deletion logic // 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) // Also prune expired tombstones (bd-08ea)
// This runs after closed issues are converted to tombstones, cleaning up old ones // This runs after closed issues are converted to tombstones, cleaning up old ones

View File

@@ -91,22 +91,36 @@ var deleteCmd = &cobra.Command{
This command will: This command will:
1. Remove all dependency links (any type, both directions) involving the issues 1. Remove all dependency links (any type, both directions) involving the issues
2. Update text references to "[deleted:ID]" in directly connected 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. This is a destructive operation that cannot be undone. Use with caution.
BATCH DELETION: BATCH DELETION:
Delete multiple issues at once: Delete multiple issues at once:
bd delete bd-1 bd-2 bd-3 --force bd delete bd-1 bd-2 bd-3 --force
Delete from file (one ID per line): Delete from file (one ID per line):
bd delete --from-file deletions.txt --force bd delete --from-file deletions.txt --force
Preview before deleting: Preview before deleting:
bd delete --from-file deletions.txt --dry-run bd delete --from-file deletions.txt --dry-run
DEPENDENCY HANDLING: DEPENDENCY HANDLING:
Default: Fails if any issue has dependents not in deletion set Default: Fails if any issue has dependents not in deletion set
bd delete bd-1 bd-2 bd delete bd-1 bd-2
Cascade: Recursively delete all dependents Cascade: Recursively delete all dependents
bd delete bd-1 --cascade --force bd delete bd-1 --cascade --force
Force: Delete and orphan dependents 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), Args: cobra.MinimumNArgs(0),
Run: func(cmd *cobra.Command, args []string) { Run: func(cmd *cobra.Command, args []string) {
CheckReadonly("delete") CheckReadonly("delete")
@@ -114,6 +128,7 @@ Force: Delete and orphan dependents
force, _ := cmd.Flags().GetBool("force") force, _ := cmd.Flags().GetBool("force")
dryRun, _ := cmd.Flags().GetBool("dry-run") dryRun, _ := cmd.Flags().GetBool("dry-run")
cascade, _ := cmd.Flags().GetBool("cascade") cascade, _ := cmd.Flags().GetBool("cascade")
hardDelete, _ := cmd.Flags().GetBool("hard")
// Use global jsonOutput set by PersistentPreRun // Use global jsonOutput set by PersistentPreRun
// Collect issue IDs from args and/or file // Collect issue IDs from args and/or file
issueIDs := make([]string, 0, len(args)) issueIDs := make([]string, 0, len(args))
@@ -150,7 +165,7 @@ Force: Delete and orphan dependents
// Handle batch deletion in direct mode // Handle batch deletion in direct mode
if len(issueIDs) > 1 { if len(issueIDs) > 1 {
deleteBatch(cmd, issueIDs, force, dryRun, cascade, jsonOutput, "batch delete") deleteBatch(cmd, issueIDs, force, dryRun, cascade, jsonOutput, hardDelete, "batch delete")
return return
} }
@@ -409,7 +424,7 @@ func removeIssueFromJSONL(issueID string) error {
} }
// deleteBatch handles deletion of multiple issues // deleteBatch handles deletion of multiple issues
//nolint:unparam // cmd parameter required for potential future use //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 // Ensure we have a direct store
if store == nil { if store == nil {
if err := ensureStoreActive(); err != 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) result, err := d.DeleteIssues(ctx, issueIDs, cascade, force, false)
if err != nil { if err != nil {
fmt.Fprintf(os.Stderr, "Error: %v\n", err) fmt.Fprintf(os.Stderr, "Error: %v\n", err)
os.Exit(1) 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) // Update text references in connected issues (using pre-collected issues)
updatedCount := updateTextReferencesInIssues(ctx, issueIDs, connectedIssues) updatedCount := updateTextReferencesInIssues(ctx, issueIDs, connectedIssues)
// Note: No longer remove from JSONL - tombstones will be exported to JSONL (bd-3b4) // 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().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("dry-run", false, "Preview what would be deleted without making changes")
deleteCmd.Flags().Bool("cascade", false, "Recursively delete all dependent issues") 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) rootCmd.AddCommand(deleteCmd)
} }

View File

@@ -492,7 +492,17 @@ func upsertIssues(ctx context.Context, sqliteStore *sqlite.SQLiteStorage, issues
continue continue
} }
seenHashes[hash] = true 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) // Phase 0: Match by external_ref first (if present)
// This enables re-syncing from external systems (Jira, GitHub, Linear) // This enables re-syncing from external systems (Jira, GitHub, Linear)
if incoming.ExternalRef != nil && *incoming.ExternalRef != "" { if incoming.ExternalRef != nil && *incoming.ExternalRef != "" {

View File

@@ -98,7 +98,10 @@ func (i *Issue) IsTombstone() bool {
// IsExpired returns true if the tombstone has exceeded its TTL. // IsExpired returns true if the tombstone has exceeded its TTL.
// Non-tombstone issues always return false. // 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 { func (i *Issue) IsExpired(ttl time.Duration) bool {
// Non-tombstones never expire // Non-tombstones never expire
if !i.IsTombstone() { if !i.IsTombstone() {
@@ -110,13 +113,22 @@ func (i *Issue) IsExpired(ttl time.Duration) bool {
return false return false
} }
// Negative TTL means "immediately expired" - for --hard mode (bd-4q8 fix)
if ttl < 0 {
return true
}
// Use default TTL if not specified // Use default TTL if not specified
if ttl == 0 { if ttl == 0 {
ttl = DefaultTombstoneTTL ttl = DefaultTombstoneTTL
} }
// Add clock skew grace period to the TTL // Only add clock skew grace period for normal TTLs (> 1 hour).
effectiveTTL := ttl + ClockSkewGrace // For short TTLs (testing/development), skip grace period.
effectiveTTL := ttl
if ttl > ClockSkewGrace {
effectiveTTL = ttl + ClockSkewGrace
}
// Check if the tombstone has exceeded its TTL // Check if the tombstone has exceeded its TTL
expirationTime := i.DeletedAt.Add(effectiveTTL) expirationTime := i.DeletedAt.Add(effectiveTTL)

View File

@@ -756,6 +756,31 @@ func TestIsExpired(t *testing.T) {
ttl: 7 * 24 * time.Hour, ttl: 7 * 24 * time.Hour,
expired: false, 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 { for _, tt := range tests {