From fe4f851d33a2deeec2934d89f7c9ff401817b1bf Mon Sep 17 00:00:00 2001 From: Steve Yegge Date: Fri, 28 Nov 2025 17:43:21 -0800 Subject: [PATCH] Fix validatePostImport false positive for legitimate deletions (bd-pg1) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The sync validation was incorrectly triggering 'data loss detected' when issue count decreased after import, even for legitimate deletions recorded in deletions.jsonl. Changes: - Modified validatePostImport to accept jsonlPath and check deletions manifest - When issue count decreases, check if decrease is within recorded deletions - Updated all call sites in sync.go and daemon_sync.go - Added comprehensive tests for deletion-aware validation 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude --- cmd/bd/daemon_sync.go | 4 +-- cmd/bd/integrity.go | 42 +++++++++++++++++++++++-- cmd/bd/integrity_test.go | 66 +++++++++++++++++++++++++++++++++++++--- cmd/bd/sync.go | 2 +- 4 files changed, 103 insertions(+), 11 deletions(-) diff --git a/cmd/bd/daemon_sync.go b/cmd/bd/daemon_sync.go index ad2e00ea..9c390f0f 100644 --- a/cmd/bd/daemon_sync.go +++ b/cmd/bd/daemon_sync.go @@ -550,7 +550,7 @@ func createAutoImportFunc(ctx context.Context, store storage.Storage, log daemon return } - if err := validatePostImport(beforeCount, afterCount); err != nil { + if err := validatePostImport(beforeCount, afterCount, jsonlPath); err != nil { log.log("Post-import validation failed: %v", err) return } @@ -743,7 +743,7 @@ func createSyncFunc(ctx context.Context, store storage.Storage, autoCommit, auto return } - if err := validatePostImport(beforeCount, afterCount); err != nil { + if err := validatePostImport(beforeCount, afterCount, jsonlPath); err != nil { log.log("Post-import validation failed: %v", err) return } diff --git a/cmd/bd/integrity.go b/cmd/bd/integrity.go index 18ce1364..8125a39b 100644 --- a/cmd/bd/integrity.go +++ b/cmd/bd/integrity.go @@ -13,6 +13,7 @@ import ( "sort" "strings" + "github.com/steveyegge/beads/internal/deletions" "github.com/steveyegge/beads/internal/storage" "github.com/steveyegge/beads/internal/types" ) @@ -287,10 +288,45 @@ func checkOrphanedDeps(ctx context.Context, store storage.Storage) ([]string, er } // validatePostImport checks that import didn't cause data loss. -// Returns error if issue count decreased (data loss) or nil if OK. -func validatePostImport(before, after int) error { +// Returns error if issue count decreased unexpectedly (data loss) or nil if OK. +// A decrease is legitimate if it matches deletions recorded in deletions.jsonl. +// +// Parameters: +// - before: issue count in DB before import +// - after: issue count in DB after import +// - jsonlPath: path to issues.jsonl (used to locate deletions.jsonl) +func validatePostImport(before, after int, jsonlPath string) error { if after < before { - return fmt.Errorf("import reduced issue count: %d → %d (data loss detected!)", before, after) + // Count decrease - check if this matches legitimate deletions + decrease := before - after + + // Load deletions manifest to check for legitimate deletions + beadsDir := filepath.Dir(jsonlPath) + deletionsPath := deletions.DefaultPath(beadsDir) + loadResult, err := deletions.LoadDeletions(deletionsPath) + if err != nil { + // If we can't load deletions, assume the worst + return fmt.Errorf("import reduced issue count: %d → %d (data loss detected! failed to verify deletions: %v)", before, after, err) + } + + // If there are deletions recorded, the decrease is likely legitimate + // We can't perfectly match because we don't know exactly which issues + // were deleted in this sync cycle vs previously. But if there are ANY + // deletions recorded and the decrease is reasonable, allow it. + numDeletions := len(loadResult.Records) + if numDeletions > 0 && decrease <= numDeletions { + // Legitimate deletion - decrease is accounted for by deletions manifest + fmt.Fprintf(os.Stderr, "Import complete: %d → %d issues (-%d, accounted for by %d deletion(s))\n", + before, after, decrease, numDeletions) + return nil + } + + // Decrease exceeds recorded deletions - potential data loss + if numDeletions > 0 { + return fmt.Errorf("import reduced issue count: %d → %d (-%d exceeds %d recorded deletion(s) - potential data loss!)", + before, after, decrease, numDeletions) + } + return fmt.Errorf("import reduced issue count: %d → %d (data loss detected! no deletions recorded)", before, after) } if after == before { fmt.Fprintf(os.Stderr, "Import complete: no changes\n") diff --git a/cmd/bd/integrity_test.go b/cmd/bd/integrity_test.go index a5a3c906..24bf75ed 100644 --- a/cmd/bd/integrity_test.go +++ b/cmd/bd/integrity_test.go @@ -163,22 +163,78 @@ func TestValidatePreExportSuite(t *testing.T) { } func TestValidatePostImport(t *testing.T) { - t.Run("issue count decreased fails", func(t *testing.T) { - err := validatePostImport(10, 5) + t.Run("issue count decreased with no deletions fails", func(t *testing.T) { + tmpDir := t.TempDir() + jsonlPath := filepath.Join(tmpDir, "issues.jsonl") + // No deletions.jsonl file exists + err := validatePostImport(10, 5, jsonlPath) if err == nil { - t.Error("Expected error for decreased issue count, got nil") + t.Error("Expected error for decreased issue count with no deletions, got nil") + } + if err != nil && !strings.Contains(err.Error(), "no deletions recorded") { + t.Errorf("Expected 'no deletions recorded' error, got: %v", err) + } + }) + + t.Run("issue count decreased within deletion count succeeds", func(t *testing.T) { + tmpDir := t.TempDir() + jsonlPath := filepath.Join(tmpDir, "issues.jsonl") + deletionsPath := filepath.Join(tmpDir, "deletions.jsonl") + + // Create deletions file with 5 deletions + deletionsContent := `{"id":"del-1","ts":"2024-01-01T00:00:00Z","by":"test"} +{"id":"del-2","ts":"2024-01-01T00:00:00Z","by":"test"} +{"id":"del-3","ts":"2024-01-01T00:00:00Z","by":"test"} +{"id":"del-4","ts":"2024-01-01T00:00:00Z","by":"test"} +{"id":"del-5","ts":"2024-01-01T00:00:00Z","by":"test"} +` + if err := os.WriteFile(deletionsPath, []byte(deletionsContent), 0600); err != nil { + t.Fatalf("Failed to write deletions file: %v", err) + } + + // Decrease of 5 matches the 5 recorded deletions + err := validatePostImport(10, 5, jsonlPath) + if err != nil { + t.Errorf("Expected no error when decrease matches deletions, got: %v", err) + } + }) + + t.Run("issue count decreased exceeding deletion count fails", func(t *testing.T) { + tmpDir := t.TempDir() + jsonlPath := filepath.Join(tmpDir, "issues.jsonl") + deletionsPath := filepath.Join(tmpDir, "deletions.jsonl") + + // Create deletions file with only 2 deletions + deletionsContent := `{"id":"del-1","ts":"2024-01-01T00:00:00Z","by":"test"} +{"id":"del-2","ts":"2024-01-01T00:00:00Z","by":"test"} +` + if err := os.WriteFile(deletionsPath, []byte(deletionsContent), 0600); err != nil { + t.Fatalf("Failed to write deletions file: %v", err) + } + + // Decrease of 5 exceeds the 2 recorded deletions + err := validatePostImport(10, 5, jsonlPath) + if err == nil { + t.Error("Expected error when decrease exceeds deletions, got nil") + } + if err != nil && !strings.Contains(err.Error(), "exceeds") { + t.Errorf("Expected 'exceeds' error, got: %v", err) } }) t.Run("issue count same succeeds", func(t *testing.T) { - err := validatePostImport(10, 10) + tmpDir := t.TempDir() + jsonlPath := filepath.Join(tmpDir, "issues.jsonl") + err := validatePostImport(10, 10, jsonlPath) if err != nil { t.Errorf("Expected no error for same count, got: %v", err) } }) t.Run("issue count increased succeeds", func(t *testing.T) { - err := validatePostImport(10, 15) + tmpDir := t.TempDir() + jsonlPath := filepath.Join(tmpDir, "issues.jsonl") + err := validatePostImport(10, 15, jsonlPath) if err != nil { t.Errorf("Expected no error for increased count, got: %v", err) } diff --git a/cmd/bd/sync.go b/cmd/bd/sync.go index 699bd7de..47c39a21 100644 --- a/cmd/bd/sync.go +++ b/cmd/bd/sync.go @@ -332,7 +332,7 @@ Use --merge to merge the sync branch back to main branch.`, if err != nil { fmt.Fprintf(os.Stderr, "Warning: failed to count issues after import: %v\n", err) } else { - if err := validatePostImport(beforeCount, afterCount); err != nil { + if err := validatePostImport(beforeCount, afterCount, jsonlPath); err != nil { fmt.Fprintf(os.Stderr, "Post-import validation failed: %v\n", err) os.Exit(1) }