fix(sync,blocked): add safety guards for issue deletion and include status=blocked in bd blocked
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 <noreply@anthropic.com>
This commit is contained in:
@@ -359,12 +359,16 @@ This usually means:
|
|||||||
3. Database corruption
|
3. Database corruption
|
||||||
4. bd was upgraded and URL canonicalization changed
|
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:
|
Solutions:
|
||||||
- If remote URL changed: bd migrate --update-repo-id
|
- If remote URL changed: bd migrate --update-repo-id
|
||||||
- If bd was upgraded: bd migrate --update-repo-id
|
- If bd was upgraded: bd migrate --update-repo-id
|
||||||
- If wrong database: rm -rf .beads && bd init
|
- If wrong database: rm -rf .beads && bd init
|
||||||
- If correct database: BEADS_IGNORE_REPO_MISMATCH=1 bd daemon
|
- 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])
|
`, 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
|
// Pull from git if not in git-free mode
|
||||||
if !skipGit {
|
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
|
// Try sync branch first
|
||||||
pulled, err := syncBranchPull(importCtx, store, log)
|
pulled, err := syncBranchPull(importCtx, store, log)
|
||||||
if err != nil {
|
if err != nil {
|
||||||
|
|||||||
@@ -883,6 +883,24 @@ func purgeDeletedIssues(ctx context.Context, sqliteStore *sqlite.SQLiteStorage,
|
|||||||
}
|
}
|
||||||
|
|
||||||
if del, found := loadResult.Records[dbIssue.ID]; found {
|
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)
|
// Issue is in deletions manifest - convert to tombstone (bd-dve)
|
||||||
if err := sqliteStore.CreateTombstone(ctx, dbIssue.ID, del.Actor, del.Reason); err != nil {
|
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)
|
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 {
|
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)
|
// Backfill the deletions manifest (self-healing)
|
||||||
backfillRecord := deletions.DeletionRecord{
|
backfillRecord := deletions.DeletionRecord{
|
||||||
ID: id,
|
ID: id,
|
||||||
|
|||||||
@@ -38,12 +38,15 @@ func TestPurgeDeletedIssues(t *testing.T) {
|
|||||||
Priority: 1,
|
Priority: 1,
|
||||||
IssueType: types.TypeTask,
|
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{
|
issue2 := &types.Issue{
|
||||||
ID: "test-def",
|
ID: "test-def",
|
||||||
Title: "Issue 2",
|
Title: "Issue 2",
|
||||||
Status: types.StatusOpen,
|
Status: types.StatusClosed,
|
||||||
Priority: 1,
|
Priority: 1,
|
||||||
IssueType: types.TypeTask,
|
IssueType: types.TypeTask,
|
||||||
|
ClosedAt: &closedTime,
|
||||||
}
|
}
|
||||||
issue3 := &types.Issue{
|
issue3 := &types.Issue{
|
||||||
ID: "test-ghi",
|
ID: "test-ghi",
|
||||||
|
|||||||
@@ -238,22 +238,38 @@ func (s *SQLiteStorage) GetStaleIssues(ctx context.Context, filter types.StaleFi
|
|||||||
return issues, rows.Err()
|
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) {
|
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)
|
// Use GROUP_CONCAT to get all blocker IDs in a single query (no N+1)
|
||||||
rows, err := s.db.QueryContext(ctx, `
|
rows, err := s.db.QueryContext(ctx, `
|
||||||
SELECT
|
SELECT
|
||||||
i.id, i.title, i.description, i.design, i.acceptance_criteria, i.notes,
|
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.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,
|
i.created_at, i.updated_at, i.closed_at, i.external_ref, i.source_repo,
|
||||||
COUNT(d.depends_on_id) as blocked_by_count,
|
COALESCE(COUNT(d.depends_on_id), 0) as blocked_by_count,
|
||||||
GROUP_CONCAT(d.depends_on_id, ',') as blocker_ids
|
COALESCE(GROUP_CONCAT(d.depends_on_id, ','), '') as blocker_ids
|
||||||
FROM issues i
|
FROM issues i
|
||||||
JOIN dependencies d ON i.id = d.issue_id
|
LEFT JOIN dependencies d ON i.id = d.issue_id
|
||||||
JOIN issues blocker ON d.depends_on_id = blocker.id
|
|
||||||
WHERE i.status IN ('open', 'in_progress', 'blocked')
|
|
||||||
AND d.type = 'blocks'
|
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')
|
AND blocker.status IN ('open', 'in_progress', 'blocked')
|
||||||
|
)
|
||||||
|
WHERE i.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
|
GROUP BY i.id
|
||||||
ORDER BY i.priority ASC
|
ORDER BY i.priority ASC
|
||||||
`)
|
`)
|
||||||
@@ -303,6 +319,8 @@ func (s *SQLiteStorage) GetBlockedIssues(ctx context.Context) ([]*types.BlockedI
|
|||||||
// Parse comma-separated blocker IDs
|
// Parse comma-separated blocker IDs
|
||||||
if blockerIDsStr != "" {
|
if blockerIDsStr != "" {
|
||||||
issue.BlockedBy = strings.Split(blockerIDsStr, ",")
|
issue.BlockedBy = strings.Split(blockerIDsStr, ",")
|
||||||
|
} else {
|
||||||
|
issue.BlockedBy = []string{}
|
||||||
}
|
}
|
||||||
|
|
||||||
blocked = append(blocked, &issue)
|
blocked = append(blocked, &issue)
|
||||||
|
|||||||
Reference in New Issue
Block a user