bd sync: 2025-11-25 11:46:06
This commit is contained in:
@@ -127,7 +127,7 @@ SEE ALSO:
|
||||
}
|
||||
|
||||
// Use the existing batch deletion logic
|
||||
deleteBatch(cmd, issueIDs, force, dryRun, cascade, jsonOutput)
|
||||
deleteBatch(cmd, issueIDs, force, dryRun, cascade, jsonOutput, "cleanup")
|
||||
},
|
||||
}
|
||||
|
||||
|
||||
@@ -1,14 +1,20 @@
|
||||
package main
|
||||
|
||||
import (
|
||||
"bufio"
|
||||
"context"
|
||||
"encoding/json"
|
||||
"fmt"
|
||||
"os"
|
||||
"os/exec"
|
||||
"path/filepath"
|
||||
"regexp"
|
||||
"strings"
|
||||
"time"
|
||||
|
||||
"github.com/fatih/color"
|
||||
"github.com/spf13/cobra"
|
||||
"github.com/steveyegge/beads/internal/deletions"
|
||||
"github.com/steveyegge/beads/internal/storage/sqlite"
|
||||
"github.com/steveyegge/beads/internal/types"
|
||||
)
|
||||
@@ -62,7 +68,7 @@ Force: Delete and orphan dependents
|
||||
issueIDs = uniqueStrings(issueIDs)
|
||||
// Handle batch deletion
|
||||
if len(issueIDs) > 1 {
|
||||
deleteBatch(cmd, issueIDs, force, dryRun, cascade, jsonOutput)
|
||||
deleteBatch(cmd, issueIDs, force, dryRun, cascade, jsonOutput, "batch delete")
|
||||
return
|
||||
}
|
||||
// Single issue deletion (legacy behavior)
|
||||
@@ -161,6 +167,13 @@ Force: Delete and orphan dependents
|
||||
return
|
||||
}
|
||||
// Actually delete
|
||||
// 0. Record deletion in manifest FIRST (before any DB changes)
|
||||
// This ensures deletion propagates via git sync even if DB operations fail
|
||||
deleteActor := getActorWithGit()
|
||||
if err := recordDeletion(issueID, deleteActor, "manual delete"); err != nil {
|
||||
fmt.Fprintf(os.Stderr, "Error: failed to record deletion: %v\n", err)
|
||||
os.Exit(1)
|
||||
}
|
||||
// 1. Update text references in connected issues (all text fields)
|
||||
updatedIssueCount := 0
|
||||
for id, connIssue := range connectedIssues {
|
||||
@@ -319,7 +332,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) {
|
||||
func deleteBatch(_ *cobra.Command, issueIDs []string, force bool, dryRun bool, cascade bool, jsonOutput bool, reason string) {
|
||||
// Ensure we have a direct store when daemon lacks delete support
|
||||
if daemonClient != nil {
|
||||
if err := ensureDirectMode("daemon does not support delete command"); err != nil {
|
||||
@@ -414,6 +427,13 @@ func deleteBatch(_ *cobra.Command, issueIDs []string, force bool, dryRun bool, c
|
||||
}
|
||||
}
|
||||
}
|
||||
// Record deletions in manifest FIRST (before any DB changes)
|
||||
// This ensures deletion propagates via git sync even if DB operations fail
|
||||
deleteActor := getActorWithGit()
|
||||
if err := recordDeletions(issueIDs, deleteActor, reason); err != nil {
|
||||
fmt.Fprintf(os.Stderr, "Error: failed to record deletions: %v\n", err)
|
||||
os.Exit(1)
|
||||
}
|
||||
// Actually delete
|
||||
result, err := d.DeleteIssues(ctx, issueIDs, cascade, force, false)
|
||||
if err != nil {
|
||||
@@ -553,6 +573,71 @@ func uniqueStrings(slice []string) []string {
|
||||
}
|
||||
return result
|
||||
}
|
||||
|
||||
// getActorWithGit returns the actor for audit trail with git config fallback.
|
||||
// Priority: global actor var (from --actor flag or BD_ACTOR env) > git config user.name > $USER > "unknown"
|
||||
func getActorWithGit() string {
|
||||
// If actor is already set (from flag or env), use it
|
||||
if actor != "" && actor != "unknown" {
|
||||
return actor
|
||||
}
|
||||
|
||||
// Try git config user.name
|
||||
cmd := exec.Command("git", "config", "user.name")
|
||||
if output, err := cmd.Output(); err == nil {
|
||||
if gitUser := strings.TrimSpace(string(output)); gitUser != "" {
|
||||
return gitUser
|
||||
}
|
||||
}
|
||||
|
||||
// Fall back to USER env
|
||||
if user := os.Getenv("USER"); user != "" {
|
||||
return user
|
||||
}
|
||||
|
||||
return "unknown"
|
||||
}
|
||||
|
||||
// getDeletionsPath returns the path to the deletions manifest file.
|
||||
// Uses the same directory as the database.
|
||||
func getDeletionsPath() string {
|
||||
// Get the .beads directory from dbPath
|
||||
beadsDir := filepath.Dir(dbPath)
|
||||
return deletions.DefaultPath(beadsDir)
|
||||
}
|
||||
|
||||
// recordDeletion appends a deletion record to the deletions manifest.
|
||||
// This MUST be called BEFORE deleting from the database to ensure
|
||||
// deletion records are never lost.
|
||||
func recordDeletion(id, deleteActor, reason string) error {
|
||||
record := deletions.DeletionRecord{
|
||||
ID: id,
|
||||
Timestamp: time.Now().UTC(),
|
||||
Actor: deleteActor,
|
||||
Reason: reason,
|
||||
}
|
||||
return deletions.AppendDeletion(getDeletionsPath(), record)
|
||||
}
|
||||
|
||||
// recordDeletions appends multiple deletion records to the deletions manifest.
|
||||
// This MUST be called BEFORE deleting from the database to ensure
|
||||
// deletion records are never lost.
|
||||
func recordDeletions(ids []string, deleteActor, reason string) error {
|
||||
path := getDeletionsPath()
|
||||
for _, id := range ids {
|
||||
record := deletions.DeletionRecord{
|
||||
ID: id,
|
||||
Timestamp: time.Now().UTC(),
|
||||
Actor: deleteActor,
|
||||
Reason: reason,
|
||||
}
|
||||
if err := deletions.AppendDeletion(path, record); err != nil {
|
||||
return fmt.Errorf("failed to record deletion for %s: %w", id, err)
|
||||
}
|
||||
}
|
||||
return nil
|
||||
}
|
||||
|
||||
func init() {
|
||||
deleteCmd.Flags().BoolP("force", "f", false, "Actually delete (without this flag, shows preview)")
|
||||
deleteCmd.Flags().String("from-file", "", "Read issue IDs from file (one per line)")
|
||||
|
||||
223
cmd/bd/delete_recording_test.go
Normal file
223
cmd/bd/delete_recording_test.go
Normal file
@@ -0,0 +1,223 @@
|
||||
package main
|
||||
|
||||
import (
|
||||
"context"
|
||||
"os"
|
||||
"path/filepath"
|
||||
"testing"
|
||||
"time"
|
||||
|
||||
"github.com/steveyegge/beads/internal/deletions"
|
||||
"github.com/steveyegge/beads/internal/storage/sqlite"
|
||||
"github.com/steveyegge/beads/internal/types"
|
||||
)
|
||||
|
||||
// TestRecordDeletion tests that recordDeletion creates deletion manifest entries
|
||||
func TestRecordDeletion(t *testing.T) {
|
||||
tmpDir := t.TempDir()
|
||||
|
||||
// Set up dbPath so getDeletionsPath() works
|
||||
oldDbPath := dbPath
|
||||
dbPath = filepath.Join(tmpDir, "beads.db")
|
||||
defer func() { dbPath = oldDbPath }()
|
||||
|
||||
// Create the .beads directory
|
||||
if err := os.MkdirAll(tmpDir, 0750); err != nil {
|
||||
t.Fatalf("failed to create directory: %v", err)
|
||||
}
|
||||
|
||||
// Test recordDeletion
|
||||
err := recordDeletion("test-abc", "test-user", "test reason")
|
||||
if err != nil {
|
||||
t.Fatalf("recordDeletion failed: %v", err)
|
||||
}
|
||||
|
||||
// Verify the deletion was recorded
|
||||
deletionsPath := getDeletionsPath()
|
||||
result, err := deletions.LoadDeletions(deletionsPath)
|
||||
if err != nil {
|
||||
t.Fatalf("LoadDeletions failed: %v", err)
|
||||
}
|
||||
|
||||
if len(result.Records) != 1 {
|
||||
t.Fatalf("expected 1 deletion record, got %d", len(result.Records))
|
||||
}
|
||||
|
||||
del, found := result.Records["test-abc"]
|
||||
if !found {
|
||||
t.Fatalf("deletion record for 'test-abc' not found")
|
||||
}
|
||||
|
||||
if del.Actor != "test-user" {
|
||||
t.Errorf("expected actor 'test-user', got '%s'", del.Actor)
|
||||
}
|
||||
|
||||
if del.Reason != "test reason" {
|
||||
t.Errorf("expected reason 'test reason', got '%s'", del.Reason)
|
||||
}
|
||||
|
||||
// Timestamp should be recent (within last minute)
|
||||
if time.Since(del.Timestamp) > time.Minute {
|
||||
t.Errorf("timestamp seems too old: %v", del.Timestamp)
|
||||
}
|
||||
}
|
||||
|
||||
// TestRecordDeletions tests that recordDeletions creates multiple deletion manifest entries
|
||||
func TestRecordDeletions(t *testing.T) {
|
||||
tmpDir := t.TempDir()
|
||||
|
||||
// Set up dbPath so getDeletionsPath() works
|
||||
oldDbPath := dbPath
|
||||
dbPath = filepath.Join(tmpDir, "beads.db")
|
||||
defer func() { dbPath = oldDbPath }()
|
||||
|
||||
// Create the .beads directory
|
||||
if err := os.MkdirAll(tmpDir, 0750); err != nil {
|
||||
t.Fatalf("failed to create directory: %v", err)
|
||||
}
|
||||
|
||||
// Test recordDeletions with multiple IDs
|
||||
ids := []string{"test-abc", "test-def", "test-ghi"}
|
||||
err := recordDeletions(ids, "batch-user", "batch cleanup")
|
||||
if err != nil {
|
||||
t.Fatalf("recordDeletions failed: %v", err)
|
||||
}
|
||||
|
||||
// Verify the deletions were recorded
|
||||
deletionsPath := getDeletionsPath()
|
||||
result, err := deletions.LoadDeletions(deletionsPath)
|
||||
if err != nil {
|
||||
t.Fatalf("LoadDeletions failed: %v", err)
|
||||
}
|
||||
|
||||
if len(result.Records) != 3 {
|
||||
t.Fatalf("expected 3 deletion records, got %d", len(result.Records))
|
||||
}
|
||||
|
||||
for _, id := range ids {
|
||||
del, found := result.Records[id]
|
||||
if !found {
|
||||
t.Errorf("deletion record for '%s' not found", id)
|
||||
continue
|
||||
}
|
||||
|
||||
if del.Actor != "batch-user" {
|
||||
t.Errorf("expected actor 'batch-user' for %s, got '%s'", id, del.Actor)
|
||||
}
|
||||
|
||||
if del.Reason != "batch cleanup" {
|
||||
t.Errorf("expected reason 'batch cleanup' for %s, got '%s'", id, del.Reason)
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
// TestGetActorWithGit tests actor sourcing logic
|
||||
func TestGetActorWithGit(t *testing.T) {
|
||||
// Save original actor value
|
||||
oldActor := actor
|
||||
defer func() { actor = oldActor }()
|
||||
|
||||
// Test case 1: actor is set from flag/env
|
||||
actor = "flag-user"
|
||||
result := getActorWithGit()
|
||||
if result != "flag-user" {
|
||||
t.Errorf("expected 'flag-user' when actor is set, got '%s'", result)
|
||||
}
|
||||
|
||||
// Test case 2: actor is "unknown" - should try git config
|
||||
actor = "unknown"
|
||||
result = getActorWithGit()
|
||||
// Can't test exact result since it depends on git config, but it shouldn't be empty
|
||||
if result == "" {
|
||||
t.Errorf("expected non-empty result when actor is 'unknown'")
|
||||
}
|
||||
|
||||
// Test case 3: actor is empty - should try git config
|
||||
actor = ""
|
||||
result = getActorWithGit()
|
||||
if result == "" {
|
||||
t.Errorf("expected non-empty result when actor is empty")
|
||||
}
|
||||
}
|
||||
|
||||
// TestDeleteRecordingOrderOfOperations verifies deletion is recorded before DB delete
|
||||
func TestDeleteRecordingOrderOfOperations(t *testing.T) {
|
||||
ctx := context.Background()
|
||||
tmpDir := t.TempDir()
|
||||
|
||||
// Set up dbPath
|
||||
oldDbPath := dbPath
|
||||
dbPath = filepath.Join(tmpDir, "beads.db")
|
||||
defer func() { dbPath = oldDbPath }()
|
||||
|
||||
// Create database
|
||||
testStore, err := sqlite.New(ctx, dbPath)
|
||||
if err != nil {
|
||||
t.Fatalf("failed to create database: %v", err)
|
||||
}
|
||||
defer testStore.Close()
|
||||
|
||||
// Initialize prefix
|
||||
if err := testStore.SetConfig(ctx, "issue_prefix", "test"); err != nil {
|
||||
t.Fatalf("failed to set prefix: %v", err)
|
||||
}
|
||||
|
||||
// Create an issue
|
||||
issue := &types.Issue{
|
||||
ID: "test-delete-order",
|
||||
Title: "Test Order of Operations",
|
||||
Status: types.StatusOpen,
|
||||
Priority: 1,
|
||||
IssueType: types.TypeTask,
|
||||
}
|
||||
if err := testStore.CreateIssue(ctx, issue, "test"); err != nil {
|
||||
t.Fatalf("failed to create issue: %v", err)
|
||||
}
|
||||
|
||||
// Record deletion (simulating what delete command does)
|
||||
if err := recordDeletion(issue.ID, "test-user", "order test"); err != nil {
|
||||
t.Fatalf("recordDeletion failed: %v", err)
|
||||
}
|
||||
|
||||
// Verify record was created BEFORE any DB changes
|
||||
deletionsPath := getDeletionsPath()
|
||||
result, err := deletions.LoadDeletions(deletionsPath)
|
||||
if err != nil {
|
||||
t.Fatalf("LoadDeletions failed: %v", err)
|
||||
}
|
||||
|
||||
if _, found := result.Records[issue.ID]; !found {
|
||||
t.Error("deletion record should exist before DB deletion")
|
||||
}
|
||||
|
||||
// Now verify the issue still exists in DB (we only recorded, didn't delete)
|
||||
existing, err := testStore.GetIssue(ctx, issue.ID)
|
||||
if err != nil {
|
||||
t.Fatalf("GetIssue failed: %v", err)
|
||||
}
|
||||
if existing == nil {
|
||||
t.Error("issue should still exist in DB (we only recorded the deletion)")
|
||||
}
|
||||
|
||||
// Now delete from DB
|
||||
if err := testStore.DeleteIssue(ctx, issue.ID); err != nil {
|
||||
t.Fatalf("DeleteIssue failed: %v", err)
|
||||
}
|
||||
|
||||
// Verify both: deletion record exists AND issue is gone from DB
|
||||
result, err = deletions.LoadDeletions(deletionsPath)
|
||||
if err != nil {
|
||||
t.Fatalf("LoadDeletions failed: %v", err)
|
||||
}
|
||||
if _, found := result.Records[issue.ID]; !found {
|
||||
t.Error("deletion record should still exist after DB deletion")
|
||||
}
|
||||
|
||||
existing, err = testStore.GetIssue(ctx, issue.ID)
|
||||
if err != nil {
|
||||
t.Fatalf("GetIssue failed: %v", err)
|
||||
}
|
||||
if existing != nil {
|
||||
t.Error("issue should be gone from DB after deletion")
|
||||
}
|
||||
}
|
||||
@@ -180,6 +180,8 @@ type ImportResult struct {
|
||||
ExpectedPrefix string // Database configured prefix
|
||||
MismatchPrefixes map[string]int // Map of mismatched prefixes to count
|
||||
SkippedDependencies []string // Dependencies skipped due to FK constraint violations
|
||||
Purged int // Issues purged from DB (found in deletions manifest)
|
||||
PurgedIDs []string // IDs that were purged
|
||||
}
|
||||
|
||||
// importIssuesCore handles the core import logic used by both manual and auto-import.
|
||||
@@ -240,6 +242,8 @@ func importIssuesCore(ctx context.Context, dbPath string, store storage.Storage,
|
||||
ExpectedPrefix: result.ExpectedPrefix,
|
||||
MismatchPrefixes: result.MismatchPrefixes,
|
||||
SkippedDependencies: result.SkippedDependencies,
|
||||
Purged: result.Purged,
|
||||
PurgedIDs: result.PurgedIDs,
|
||||
}, nil
|
||||
}
|
||||
|
||||
|
||||
Reference in New Issue
Block a user