From 8e58c306a7015d123f9f884bd9488fd8471d727e Mon Sep 17 00:00:00 2001 From: Steve Yegge Date: Sun, 14 Dec 2025 23:07:54 -0800 Subject: [PATCH] fix(sync,blocked): add safety guards for issue deletion and include status=blocked in bd blocked MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit GH#464: Add safety guards to prevent deletion of open/in_progress issues during sync: - Safety guard in git-history-backfill (importer.go) - Safety guard in deletions manifest processing - Warning when uncommitted changes detected before pull (daemon_sync.go) - Enhanced repo ID mismatch error message GH#545: Fix bd blocked to show status=blocked issues (sqlite/ready.go): - Changed from INNER JOIN to LEFT JOIN to include issues without dependencies - Added WHERE clause to include both status=blocked AND dependency-blocked issues 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Opus 4.5 --- cmd/bd/daemon_sync.go | 18 +++++++++++++++- internal/importer/importer.go | 36 ++++++++++++++++++++++++++++++++ internal/importer/purge_test.go | 5 ++++- internal/storage/sqlite/ready.go | 32 +++++++++++++++++++++------- 4 files changed, 82 insertions(+), 9 deletions(-) diff --git a/cmd/bd/daemon_sync.go b/cmd/bd/daemon_sync.go index 3a264021..8bbcf0d9 100644 --- a/cmd/bd/daemon_sync.go +++ b/cmd/bd/daemon_sync.go @@ -359,12 +359,16 @@ This usually means: 3. Database corruption 4. bd was upgraded and URL canonicalization changed +⚠️ CRITICAL: This mismatch can cause beads to incorrectly delete issues during sync! + The git-history-backfill mechanism may treat your local issues as deleted + because they don't exist in the remote repository's history. + Solutions: - If remote URL changed: bd migrate --update-repo-id - If bd was upgraded: bd migrate --update-repo-id - If wrong database: rm -rf .beads && bd init - If correct database: BEADS_IGNORE_REPO_MISMATCH=1 bd daemon - (Warning: This can cause data corruption across clones!) + (Warning: This can cause data corruption and unwanted deletions across clones!) `, storedRepoID[:8], currentRepoID[:8]) } @@ -558,6 +562,18 @@ func performAutoImport(ctx context.Context, store storage.Storage, skipGit bool, // Pull from git if not in git-free mode if !skipGit { + // SAFETY CHECK (bd-k92d): Warn if there are uncommitted local changes + // This helps detect race conditions where local work hasn't been pushed yet + jsonlPath := findJSONLPath() + if jsonlPath != "" { + if hasLocalChanges, err := gitHasChanges(importCtx, jsonlPath); err == nil && hasLocalChanges { + log.log("⚠️ WARNING: Uncommitted local changes detected in %s", jsonlPath) + log.log(" Pulling from remote may overwrite local unpushed changes.") + log.log(" Consider running 'bd sync' to commit and push your changes first.") + // Continue anyway, but user has been warned + } + } + // Try sync branch first pulled, err := syncBranchPull(importCtx, store, log) if err != nil { diff --git a/internal/importer/importer.go b/internal/importer/importer.go index b014a95b..d7804883 100644 --- a/internal/importer/importer.go +++ b/internal/importer/importer.go @@ -883,6 +883,24 @@ func purgeDeletedIssues(ctx context.Context, sqliteStore *sqlite.SQLiteStorage, } if del, found := loadResult.Records[dbIssue.ID]; found { + // SAFETY GUARD (bd-k92d): Prevent deletion of open/in_progress issues without explicit warning + // This protects against data loss from: + // 1. Repo ID mismatches causing incorrect deletions + // 2. Race conditions during daemon sync + // 3. Accidental deletion of active work + if dbIssue.Status == types.StatusOpen || dbIssue.Status == types.StatusInProgress { + fmt.Fprintf(os.Stderr, "⚠️ WARNING: Refusing to delete %s with status=%s\n", dbIssue.ID, dbIssue.Status) + fmt.Fprintf(os.Stderr, " Title: %s\n", dbIssue.Title) + fmt.Fprintf(os.Stderr, " This issue is in deletions.jsonl but still open/in_progress in your database.\n") + fmt.Fprintf(os.Stderr, " This may indicate:\n") + fmt.Fprintf(os.Stderr, " - A repo ID mismatch (check with 'bd migrate --update-repo-id')\n") + fmt.Fprintf(os.Stderr, " - A sync race condition with unpushed local changes\n") + fmt.Fprintf(os.Stderr, " - Accidental deletion on another clone\n") + fmt.Fprintf(os.Stderr, " To force deletion: bd delete %s\n", dbIssue.ID) + fmt.Fprintf(os.Stderr, " To keep this issue: remove it from .beads/deletions.jsonl\n\n") + continue + } + // 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) @@ -950,6 +968,24 @@ func purgeDeletedIssues(ctx context.Context, sqliteStore *sqlite.SQLiteStorage, } for _, id := range deletedViaGit { + // SAFETY GUARD (bd-k92d): Check if this is an open/in_progress issue before deleting + // Get the issue from database to check its status + issue, err := sqliteStore.GetIssue(ctx, id) + if err == nil && issue != nil { + if issue.Status == types.StatusOpen || issue.Status == types.StatusInProgress { + fmt.Fprintf(os.Stderr, "⚠️ WARNING: git-history-backfill refusing to delete %s with status=%s\n", id, issue.Status) + fmt.Fprintf(os.Stderr, " Title: %s\n", issue.Title) + fmt.Fprintf(os.Stderr, " This issue was found in git history but is still open/in_progress.\n") + fmt.Fprintf(os.Stderr, " This may indicate:\n") + fmt.Fprintf(os.Stderr, " - A repo ID mismatch between clones\n") + fmt.Fprintf(os.Stderr, " - The issue was re-created after being deleted\n") + fmt.Fprintf(os.Stderr, " - Local uncommitted work that conflicts with remote history\n") + fmt.Fprintf(os.Stderr, " To force deletion: bd delete %s\n", id) + fmt.Fprintf(os.Stderr, " To prevent git-history checks: use --no-git-history flag\n\n") + continue + } + } + // Backfill the deletions manifest (self-healing) backfillRecord := deletions.DeletionRecord{ ID: id, diff --git a/internal/importer/purge_test.go b/internal/importer/purge_test.go index 2ed5b45b..656c4f2b 100644 --- a/internal/importer/purge_test.go +++ b/internal/importer/purge_test.go @@ -38,12 +38,15 @@ func TestPurgeDeletedIssues(t *testing.T) { Priority: 1, IssueType: types.TypeTask, } + // issue2 is CLOSED so it can be safely deleted (bd-k92d: safety guard prevents deleting open/in_progress) + closedTime := time.Now().UTC() issue2 := &types.Issue{ ID: "test-def", Title: "Issue 2", - Status: types.StatusOpen, + Status: types.StatusClosed, Priority: 1, IssueType: types.TypeTask, + ClosedAt: &closedTime, } issue3 := &types.Issue{ ID: "test-ghi", diff --git a/internal/storage/sqlite/ready.go b/internal/storage/sqlite/ready.go index 1865a96f..959a11f5 100644 --- a/internal/storage/sqlite/ready.go +++ b/internal/storage/sqlite/ready.go @@ -238,22 +238,38 @@ func (s *SQLiteStorage) GetStaleIssues(ctx context.Context, filter types.StaleFi return issues, rows.Err() } -// GetBlockedIssues returns issues that are blocked by dependencies +// GetBlockedIssues returns issues that are blocked by dependencies or have status=blocked func (s *SQLiteStorage) GetBlockedIssues(ctx context.Context) ([]*types.BlockedIssue, error) { + // Use UNION to combine: + // 1. Issues with open/in_progress/blocked status that have dependency blockers + // 2. Issues with status=blocked (even if they have no dependency blockers) // Use GROUP_CONCAT to get all blocker IDs in a single query (no N+1) rows, err := s.db.QueryContext(ctx, ` SELECT i.id, i.title, i.description, i.design, i.acceptance_criteria, i.notes, i.status, i.priority, i.issue_type, i.assignee, i.estimated_minutes, i.created_at, i.updated_at, i.closed_at, i.external_ref, i.source_repo, - COUNT(d.depends_on_id) as blocked_by_count, - GROUP_CONCAT(d.depends_on_id, ',') as blocker_ids + COALESCE(COUNT(d.depends_on_id), 0) as blocked_by_count, + COALESCE(GROUP_CONCAT(d.depends_on_id, ','), '') as blocker_ids FROM issues i - JOIN dependencies d ON i.id = d.issue_id - JOIN issues blocker ON d.depends_on_id = blocker.id + LEFT JOIN dependencies d ON i.id = d.issue_id + AND d.type = 'blocks' + AND EXISTS ( + SELECT 1 FROM issues blocker + WHERE blocker.id = d.depends_on_id + AND blocker.status IN ('open', 'in_progress', 'blocked') + ) WHERE i.status IN ('open', 'in_progress', 'blocked') - AND d.type = 'blocks' - AND blocker.status IN ('open', 'in_progress', 'blocked') + AND ( + i.status = 'blocked' + OR EXISTS ( + SELECT 1 FROM dependencies d2 + JOIN issues blocker ON d2.depends_on_id = blocker.id + WHERE d2.issue_id = i.id + AND d2.type = 'blocks' + AND blocker.status IN ('open', 'in_progress', 'blocked') + ) + ) GROUP BY i.id ORDER BY i.priority ASC `) @@ -303,6 +319,8 @@ func (s *SQLiteStorage) GetBlockedIssues(ctx context.Context) ([]*types.BlockedI // Parse comma-separated blocker IDs if blockerIDsStr != "" { issue.BlockedBy = strings.Split(blockerIDsStr, ",") + } else { + issue.BlockedBy = []string{} } blocked = append(blocked, &issue)