Fix bd-9bsx: Add sync validation to prevent infinite dirty loop
- Added dbNeedsExport() to check if DB and JSONL are in sync - Only re-export after import if DB has changes that differ from JSONL - Prevents unconditional re-export that caused infinite dirty state - Added comprehensive tests for sync validation Fixes recurring dirty state after merge conflicts that plagued users for weeks. Amp-Thread-ID: https://ampcode.com/threads/T-f4f8c8c6-07bc-4334-9109-4626b4fd7a24 Co-authored-by: Amp <amp@ampcode.com>
This commit is contained in:
Steve Yegge
@@ -223,3 +223,49 @@ func countDBIssuesFast(ctx context.Context, store storage.Storage) (int, error)
|
||||
}
|
||||
return len(issues), nil
|
||||
}
|
||||
|
||||
// dbNeedsExport checks if the database has changes that differ from JSONL.
|
||||
// Returns true if export is needed, false if DB and JSONL are already in sync.
|
||||
func dbNeedsExport(ctx context.Context, store storage.Storage, jsonlPath string) (bool, error) {
|
||||
// Check if JSONL exists
|
||||
jsonlInfo, err := os.Stat(jsonlPath)
|
||||
if os.IsNotExist(err) {
|
||||
// JSONL doesn't exist - always need to export
|
||||
return true, nil
|
||||
}
|
||||
if err != nil {
|
||||
return false, fmt.Errorf("failed to stat JSONL: %w", err)
|
||||
}
|
||||
|
||||
// Check database modification time
|
||||
beadsDir := filepath.Dir(jsonlPath)
|
||||
dbPath := filepath.Join(beadsDir, "beads.db")
|
||||
dbInfo, err := os.Stat(dbPath)
|
||||
if err != nil {
|
||||
return false, fmt.Errorf("failed to stat database: %w", err)
|
||||
}
|
||||
|
||||
// If database is newer than JSONL, we need to export
|
||||
if dbInfo.ModTime().After(jsonlInfo.ModTime()) {
|
||||
return true, nil
|
||||
}
|
||||
|
||||
// If modification times suggest they're in sync, verify counts match
|
||||
dbCount, err := countDBIssuesFast(ctx, store)
|
||||
if err != nil {
|
||||
return false, fmt.Errorf("failed to count database issues: %w", err)
|
||||
}
|
||||
|
||||
jsonlCount, err := countIssuesInJSONL(jsonlPath)
|
||||
if err != nil {
|
||||
return false, fmt.Errorf("failed to count JSONL issues: %w", err)
|
||||
}
|
||||
|
||||
// If counts don't match, we need to export
|
||||
if dbCount != jsonlCount {
|
||||
return true, nil
|
||||
}
|
||||
|
||||
// DB and JSONL appear to be in sync
|
||||
return false, nil
|
||||
}
|
||||
|
||||
@@ -214,27 +214,40 @@ Use --merge to merge the sync branch back to main branch.`,
|
||||
}
|
||||
}
|
||||
|
||||
// Step 4.5: Export back to JSONL if import made changes (e.g., rename detection)
|
||||
// This ensures JSONL stays in sync with DB after rename detection
|
||||
fmt.Println("→ Re-exporting after import to sync rename changes...")
|
||||
if err := exportToJSONL(ctx, jsonlPath); err != nil {
|
||||
fmt.Fprintf(os.Stderr, "Error re-exporting after import: %v\n", err)
|
||||
os.Exit(1)
|
||||
}
|
||||
|
||||
// Step 4.6: Commit the re-export if it created changes
|
||||
hasPostImportChanges, err := gitHasChanges(ctx, jsonlPath)
|
||||
if err != nil {
|
||||
fmt.Fprintf(os.Stderr, "Error checking git status after re-export: %v\n", err)
|
||||
os.Exit(1)
|
||||
}
|
||||
if hasPostImportChanges {
|
||||
fmt.Println("→ Committing rename detection changes...")
|
||||
if err := gitCommit(ctx, jsonlPath, "bd sync: apply rename detection from import"); err != nil {
|
||||
fmt.Fprintf(os.Stderr, "Error committing post-import changes: %v\n", err)
|
||||
os.Exit(1)
|
||||
// Step 4.5: Check if DB needs re-export (only if DB differs from JSONL)
|
||||
// This prevents the infinite loop: import → export → commit → dirty again
|
||||
if err := ensureStoreActive(); err == nil && store != nil {
|
||||
needsExport, err := dbNeedsExport(ctx, store, jsonlPath)
|
||||
if err != nil {
|
||||
fmt.Fprintf(os.Stderr, "Warning: failed to check if export needed: %v\n", err)
|
||||
// Conservative: assume export needed
|
||||
needsExport = true
|
||||
}
|
||||
|
||||
if needsExport {
|
||||
fmt.Println("→ Re-exporting after import to sync DB changes...")
|
||||
if err := exportToJSONL(ctx, jsonlPath); err != nil {
|
||||
fmt.Fprintf(os.Stderr, "Error re-exporting after import: %v\n", err)
|
||||
os.Exit(1)
|
||||
}
|
||||
|
||||
// Step 4.6: Commit the re-export if it created changes
|
||||
hasPostImportChanges, err := gitHasChanges(ctx, jsonlPath)
|
||||
if err != nil {
|
||||
fmt.Fprintf(os.Stderr, "Error checking git status after re-export: %v\n", err)
|
||||
os.Exit(1)
|
||||
}
|
||||
if hasPostImportChanges {
|
||||
fmt.Println("→ Committing DB changes from import...")
|
||||
if err := gitCommit(ctx, jsonlPath, "bd sync: apply DB changes after import"); err != nil {
|
||||
fmt.Fprintf(os.Stderr, "Error committing post-import changes: %v\n", err)
|
||||
os.Exit(1)
|
||||
}
|
||||
hasChanges = true // Mark that we have changes to push
|
||||
}
|
||||
} else {
|
||||
fmt.Println("→ DB and JSONL in sync, skipping re-export")
|
||||
}
|
||||
hasChanges = true // Mark that we have changes to push
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
220
cmd/bd/sync_merge_test.go
Normal file
220
cmd/bd/sync_merge_test.go
Normal file
@@ -0,0 +1,220 @@
|
||||
package main
|
||||
|
||||
import (
|
||||
"context"
|
||||
"os"
|
||||
"path/filepath"
|
||||
"testing"
|
||||
"time"
|
||||
|
||||
"github.com/steveyegge/beads/internal/storage/sqlite"
|
||||
"github.com/steveyegge/beads/internal/types"
|
||||
)
|
||||
|
||||
// setupTestStore creates a test storage with issue_prefix configured
|
||||
func setupTestStore(t *testing.T, dbPath string) *sqlite.SQLiteStorage {
|
||||
t.Helper()
|
||||
|
||||
store, err := sqlite.New(dbPath)
|
||||
if err != nil {
|
||||
t.Fatalf("Failed to create store: %v", err)
|
||||
}
|
||||
|
||||
ctx := context.Background()
|
||||
if err := store.SetConfig(ctx, "issue_prefix", "bd"); err != nil {
|
||||
store.Close()
|
||||
t.Fatalf("Failed to set issue_prefix: %v", err)
|
||||
}
|
||||
|
||||
return store
|
||||
}
|
||||
|
||||
// TestDBNeedsExport_InSync verifies dbNeedsExport returns false when DB and JSONL are in sync
|
||||
func TestDBNeedsExport_InSync(t *testing.T) {
|
||||
tmpDir := t.TempDir()
|
||||
dbPath := filepath.Join(tmpDir, "beads.db")
|
||||
jsonlPath := filepath.Join(tmpDir, "beads.jsonl")
|
||||
|
||||
store := setupTestStore(t, dbPath)
|
||||
defer store.Close()
|
||||
|
||||
ctx := context.Background()
|
||||
|
||||
// Create an issue in DB
|
||||
issue := &types.Issue{
|
||||
Title: "Test Issue",
|
||||
Status: types.StatusOpen,
|
||||
Priority: 1,
|
||||
IssueType: types.TypeBug,
|
||||
}
|
||||
err := store.CreateIssue(ctx, issue, "test-user")
|
||||
if err != nil {
|
||||
t.Fatalf("Failed to create issue: %v", err)
|
||||
}
|
||||
|
||||
// Export to JSONL
|
||||
if err := exportToJSONLWithStore(ctx, store, jsonlPath); err != nil {
|
||||
t.Fatalf("Failed to export: %v", err)
|
||||
}
|
||||
|
||||
// Wait a moment to ensure DB mtime isn't newer
|
||||
time.Sleep(10 * time.Millisecond)
|
||||
|
||||
// Touch JSONL to make it newer than DB
|
||||
now := time.Now()
|
||||
if err := os.Chtimes(jsonlPath, now, now); err != nil {
|
||||
t.Fatalf("Failed to touch JSONL: %v", err)
|
||||
}
|
||||
|
||||
// DB and JSONL should be in sync
|
||||
needsExport, err := dbNeedsExport(ctx, store, jsonlPath)
|
||||
if err != nil {
|
||||
t.Fatalf("dbNeedsExport failed: %v", err)
|
||||
}
|
||||
|
||||
if needsExport {
|
||||
t.Errorf("Expected needsExport=false (DB and JSONL in sync), got true")
|
||||
}
|
||||
}
|
||||
|
||||
// TestDBNeedsExport_DBNewer verifies dbNeedsExport returns true when DB is modified
|
||||
func TestDBNeedsExport_DBNewer(t *testing.T) {
|
||||
tmpDir := t.TempDir()
|
||||
dbPath := filepath.Join(tmpDir, "beads.db")
|
||||
jsonlPath := filepath.Join(tmpDir, "beads.jsonl")
|
||||
|
||||
store := setupTestStore(t, dbPath)
|
||||
defer store.Close()
|
||||
|
||||
ctx := context.Background()
|
||||
|
||||
// Create and export issue
|
||||
issue1 := &types.Issue{
|
||||
Title: "Test Issue",
|
||||
Status: types.StatusOpen,
|
||||
Priority: 1,
|
||||
IssueType: types.TypeBug,
|
||||
}
|
||||
err := store.CreateIssue(ctx, issue1, "test-user")
|
||||
if err != nil {
|
||||
t.Fatalf("Failed to create issue: %v", err)
|
||||
}
|
||||
|
||||
if err := exportToJSONLWithStore(ctx, store, jsonlPath); err != nil {
|
||||
t.Fatalf("Failed to export: %v", err)
|
||||
}
|
||||
|
||||
// Wait and modify DB
|
||||
time.Sleep(10 * time.Millisecond)
|
||||
issue2 := &types.Issue{
|
||||
Title: "Another Issue",
|
||||
Status: types.StatusOpen,
|
||||
Priority: 2,
|
||||
IssueType: types.TypeTask,
|
||||
}
|
||||
err = store.CreateIssue(ctx, issue2, "test-user")
|
||||
if err != nil {
|
||||
t.Fatalf("Failed to create second issue: %v", err)
|
||||
}
|
||||
|
||||
// DB is newer, should need export
|
||||
needsExport, err := dbNeedsExport(ctx, store, jsonlPath)
|
||||
if err != nil {
|
||||
t.Fatalf("dbNeedsExport failed: %v", err)
|
||||
}
|
||||
|
||||
if !needsExport {
|
||||
t.Errorf("Expected needsExport=true (DB modified), got false")
|
||||
}
|
||||
}
|
||||
|
||||
// TestDBNeedsExport_CountMismatch verifies dbNeedsExport returns true when counts differ
|
||||
func TestDBNeedsExport_CountMismatch(t *testing.T) {
|
||||
tmpDir := t.TempDir()
|
||||
dbPath := filepath.Join(tmpDir, "beads.db")
|
||||
jsonlPath := filepath.Join(tmpDir, "beads.jsonl")
|
||||
|
||||
store := setupTestStore(t, dbPath)
|
||||
defer store.Close()
|
||||
|
||||
ctx := context.Background()
|
||||
|
||||
// Create and export issue
|
||||
issue1 := &types.Issue{
|
||||
Title: "Test Issue",
|
||||
Status: types.StatusOpen,
|
||||
Priority: 1,
|
||||
IssueType: types.TypeBug,
|
||||
}
|
||||
err := store.CreateIssue(ctx, issue1, "test-user")
|
||||
if err != nil {
|
||||
t.Fatalf("Failed to create issue: %v", err)
|
||||
}
|
||||
|
||||
if err := exportToJSONLWithStore(ctx, store, jsonlPath); err != nil {
|
||||
t.Fatalf("Failed to export: %v", err)
|
||||
}
|
||||
|
||||
// Add another issue to DB but don't export
|
||||
issue2 := &types.Issue{
|
||||
Title: "Another Issue",
|
||||
Status: types.StatusOpen,
|
||||
Priority: 2,
|
||||
IssueType: types.TypeTask,
|
||||
}
|
||||
err = store.CreateIssue(ctx, issue2, "test-user")
|
||||
if err != nil {
|
||||
t.Fatalf("Failed to create second issue: %v", err)
|
||||
}
|
||||
|
||||
// Make JSONL appear newer (but counts differ)
|
||||
time.Sleep(10 * time.Millisecond)
|
||||
now := time.Now().Add(1 * time.Hour) // Way in the future
|
||||
if err := os.Chtimes(jsonlPath, now, now); err != nil {
|
||||
t.Fatalf("Failed to touch JSONL: %v", err)
|
||||
}
|
||||
|
||||
// Counts mismatch, should need export
|
||||
needsExport, err := dbNeedsExport(ctx, store, jsonlPath)
|
||||
if err != nil {
|
||||
t.Fatalf("dbNeedsExport failed: %v", err)
|
||||
}
|
||||
|
||||
if !needsExport {
|
||||
t.Errorf("Expected needsExport=true (count mismatch), got false")
|
||||
}
|
||||
}
|
||||
|
||||
// TestDBNeedsExport_NoJSONL verifies dbNeedsExport returns true when JSONL doesn't exist
|
||||
func TestDBNeedsExport_NoJSONL(t *testing.T) {
|
||||
tmpDir := t.TempDir()
|
||||
dbPath := filepath.Join(tmpDir, "beads.db")
|
||||
jsonlPath := filepath.Join(tmpDir, "beads.jsonl")
|
||||
|
||||
store := setupTestStore(t, dbPath)
|
||||
defer store.Close()
|
||||
|
||||
ctx := context.Background()
|
||||
|
||||
// Create issue but don't export
|
||||
issue := &types.Issue{
|
||||
Title: "Test Issue",
|
||||
Status: types.StatusOpen,
|
||||
Priority: 1,
|
||||
IssueType: types.TypeBug,
|
||||
}
|
||||
err := store.CreateIssue(ctx, issue, "test-user")
|
||||
if err != nil {
|
||||
t.Fatalf("Failed to create issue: %v", err)
|
||||
}
|
||||
|
||||
// JSONL doesn't exist, should need export
|
||||
needsExport, err := dbNeedsExport(ctx, store, jsonlPath)
|
||||
if err != nil {
|
||||
t.Fatalf("dbNeedsExport failed: %v", err)
|
||||
}
|
||||
|
||||
if !needsExport {
|
||||
t.Fatalf("Expected needsExport=true (JSONL missing), got false")
|
||||
}
|
||||
}
|
||||
Reference in New Issue
Block a user