fix: Fix daemon export leaving JSONL newer than database (issues #301, #321)

After daemon auto-export, JSONL mtime could be newer than database mtime due to SQLite WAL mode not updating beads.db until checkpoint. This caused validatePreExport to incorrectly block subsequent exports with "JSONL is newer than database" error, leading to daemon shutdown. Solution: Call TouchDatabaseFile after all export operations to ensure database mtime >= JSONL mtime. This prevents false positives in validation
2025-11-18 17:25:32 -05:00
parent aeea3532cd
commit 04a1996fd9
6 changed files with 298 additions and 12 deletions
--- a/cmd/bd/daemon_sync.go
+++ b/cmd/bd/daemon_sync.go
@@ -307,6 +307,14 @@ func createExportFunc(ctx context.Context, store storage.Storage, autoCommit, au
 		}
 		log.log("Exported to JSONL")

+		// Update database mtime to be >= JSONL mtime (fixes #278, #301, #321)
+		// This prevents validatePreExport from incorrectly blocking on next export
+		// with "JSONL is newer than database" after daemon auto-export
+		dbPath := filepath.Join(beadsDir, "beads.db")
+		if err := TouchDatabaseFile(dbPath, jsonlPath); err != nil {
+			log.log("Warning: failed to update database mtime: %v", err)
+		}
+
 		// Auto-commit if enabled
 		if autoCommit {
 			// Try sync branch commit first
@@ -502,6 +510,13 @@ func createSyncFunc(ctx context.Context, store storage.Storage, autoCommit, auto
 		}
 		log.log("Exported to JSONL")

+		// Update database mtime to be >= JSONL mtime (fixes #278, #301, #321)
+		// This prevents validatePreExport from incorrectly blocking on next export
+		dbPath := filepath.Join(beadsDir, "beads.db")
+		if err := TouchDatabaseFile(dbPath, jsonlPath); err != nil {
+			log.log("Warning: failed to update database mtime: %v", err)
+		}
+
 		// Capture left snapshot (pre-pull state) for 3-way merge
 		// This is mandatory for deletion tracking integrity
 		// In multi-repo mode, capture snapshots for all JSONL files
@@ -597,6 +612,12 @@ func createSyncFunc(ctx context.Context, store storage.Storage, autoCommit, auto
 		}
 		log.log("Imported from JSONL")

+		// Update database mtime after import (fixes #278, #301, #321)
+		// Sync branch import can update JSONL timestamp, so ensure DB >= JSONL
+		if err := TouchDatabaseFile(dbPath, jsonlPath); err != nil {
+			log.log("Warning: failed to update database mtime: %v", err)
+		}
+
 		// Validate import didn't cause data loss
 		afterCount, err := countDBIssues(syncCtx, store)
 		if err != nil {
--- a/cmd/bd/export.go
+++ b/cmd/bd/export.go
@@ -385,6 +385,18 @@ Output to stdout by default, or use -o flag for file output.`,
 			fmt.Fprintf(os.Stderr, "  Mismatch indicates export failed to write all issues\n")
 			os.Exit(1)
 		}
+
+			// Update database mtime to be >= JSONL mtime (fixes #278, #301, #321)
+			// Only do this when exporting to default JSONL path (not arbitrary outputs)
+			// This prevents validatePreExport from incorrectly blocking on next export
+			if output == "" || output == findJSONLPath() {
+				beadsDir := filepath.Dir(finalPath)
+				dbPath := filepath.Join(beadsDir, "beads.db")
+				if err := TouchDatabaseFile(dbPath, finalPath); err != nil {
+					// Log warning but don't fail export
+					fmt.Fprintf(os.Stderr, "Warning: failed to update database mtime: %v\n", err)
+				}
+			}
 	}

 	// Output statistics if JSON format requested
--- a/cmd/bd/export_mtime_test.go
+++ b/cmd/bd/export_mtime_test.go
@@ -0,0 +1,242 @@
+package main
+
+import (
+	"context"
+	"os"
+	"path/filepath"
+	"testing"
+	"time"
+
+	"github.com/steveyegge/beads/internal/storage/sqlite"
+	"github.com/steveyegge/beads/internal/types"
+)
+
+// TestExportUpdatesDatabaseMtime verifies that export updates database mtime
+// to be >= JSONL mtime, fixing issues #278, #301, #321
+func TestExportUpdatesDatabaseMtime(t *testing.T) {
+	if testing.Short() {
+		t.Skip("skipping slow test in short mode")
+	}
+
+	tmpDir := t.TempDir()
+	beadsDir := filepath.Join(tmpDir, ".beads")
+	if err := os.Mkdir(beadsDir, 0750); err != nil {
+		t.Fatal(err)
+	}
+
+	dbPath := filepath.Join(beadsDir, "beads.db")
+	jsonlPath := filepath.Join(beadsDir, "issues.jsonl")
+
+	// Create and populate database
+	store, err := sqlite.New(dbPath)
+	if err != nil {
+		t.Fatalf("Failed to create store: %v", err)
+	}
+	defer store.Close()
+
+	ctx := context.Background()
+
+	// Initialize database with issue_prefix
+	if err := store.SetConfig(ctx, "issue_prefix", "test"); err != nil {
+		t.Fatalf("Failed to set issue_prefix: %v", err)
+	}
+
+	// Create a test issue
+	issue := &types.Issue{
+		ID:        "test-1",
+		Title:     "Test Issue",
+		Status:    types.StatusOpen,
+		Priority:  2,
+		IssueType: types.TypeTask,
+	}
+	if err := store.CreateIssue(ctx, issue, "test-actor"); err != nil {
+		t.Fatalf("Failed to create issue: %v", err)
+	}
+
+	// Wait a bit to ensure mtime difference
+	time.Sleep(1 * time.Second)
+
+	// Export to JSONL (simulates daemon export)
+	if err := exportToJSONLWithStore(ctx, store, jsonlPath); err != nil {
+		t.Fatalf("Export failed: %v", err)
+	}
+
+	// Get JSONL mtime
+	jsonlInfo, err := os.Stat(jsonlPath)
+	if err != nil {
+		t.Fatalf("Failed to stat JSONL after export: %v", err)
+	}
+
+	// WITHOUT the fix, JSONL would be newer than DB here
+	// Simulating the old buggy behavior before calling TouchDatabaseFile
+	dbInfoAfterExport, err := os.Stat(dbPath)
+	if err != nil {
+		t.Fatalf("Failed to stat database after export: %v", err)
+	}
+
+	// In old buggy behavior, JSONL mtime > DB mtime
+	t.Logf("Before TouchDatabaseFile: DB mtime=%v, JSONL mtime=%v",
+		dbInfoAfterExport.ModTime(), jsonlInfo.ModTime())
+
+	// Now apply the fix
+	if err := TouchDatabaseFile(dbPath, jsonlPath); err != nil {
+		t.Fatalf("TouchDatabaseFile failed: %v", err)
+	}
+
+	// Get final database mtime
+	dbInfoAfterTouch, err := os.Stat(dbPath)
+	if err != nil {
+		t.Fatalf("Failed to stat database after touch: %v", err)
+	}
+
+	t.Logf("After TouchDatabaseFile: DB mtime=%v, JSONL mtime=%v",
+		dbInfoAfterTouch.ModTime(), jsonlInfo.ModTime())
+
+	// VERIFY: Database mtime should be >= JSONL mtime
+	if dbInfoAfterTouch.ModTime().Before(jsonlInfo.ModTime()) {
+		t.Errorf("Database mtime should be >= JSONL mtime after export")
+		t.Errorf("DB mtime: %v, JSONL mtime: %v",
+			dbInfoAfterTouch.ModTime(), jsonlInfo.ModTime())
+	}
+
+	// VERIFY: validatePreExport should now pass (not block on next export)
+	if err := validatePreExport(ctx, store, jsonlPath); err != nil {
+		t.Errorf("validatePreExport should pass after TouchDatabaseFile, but got error: %v", err)
+	}
+}
+
+// TestDaemonExportScenario simulates the full daemon auto-export workflow
+// that was causing issue #278 (daemon shutting down after export)
+func TestDaemonExportScenario(t *testing.T) {
+	if testing.Short() {
+		t.Skip("skipping slow test in short mode")
+	}
+
+	tmpDir := t.TempDir()
+	beadsDir := filepath.Join(tmpDir, ".beads")
+	if err := os.Mkdir(beadsDir, 0750); err != nil {
+		t.Fatal(err)
+	}
+
+	dbPath := filepath.Join(beadsDir, "beads.db")
+	jsonlPath := filepath.Join(beadsDir, "issues.jsonl")
+
+	// Create and populate database
+	store, err := sqlite.New(dbPath)
+	if err != nil {
+		t.Fatalf("Failed to create store: %v", err)
+	}
+	defer store.Close()
+
+	ctx := context.Background()
+
+	// Initialize database with issue_prefix
+	if err := store.SetConfig(ctx, "issue_prefix", "bd"); err != nil {
+		t.Fatalf("Failed to set issue_prefix: %v", err)
+	}
+
+	// Step 1: User creates an issue (e.g., bd close bd-123)
+	now := time.Now()
+	issue := &types.Issue{
+		ID:        "bd-123",
+		Title:     "User created issue",
+		Status:    types.StatusClosed,
+		Priority:  1,
+		IssueType: types.TypeTask,
+		ClosedAt:  &now,
+	}
+	if err := store.CreateIssue(ctx, issue, "test-user"); err != nil {
+		t.Fatalf("Failed to create issue: %v", err)
+	}
+
+	// Database is now newer than JSONL (JSONL doesn't exist yet)
+	time.Sleep(1 * time.Second)
+
+	// Step 2: Daemon auto-exports after delay (30s-4min in real scenario)
+	// This simulates the daemon's export cycle
+	if err := exportToJSONLWithStore(ctx, store, jsonlPath); err != nil {
+		t.Fatalf("Daemon export failed: %v", err)
+	}
+
+	// THIS IS THE FIX: daemon now calls TouchDatabaseFile after export
+	if err := TouchDatabaseFile(dbPath, jsonlPath); err != nil {
+		t.Fatalf("TouchDatabaseFile failed: %v", err)
+	}
+
+	// Step 3: User runs bd sync shortly after
+	// WITHOUT the fix, this would fail with "JSONL is newer than database"
+	// WITH the fix, this should succeed
+	if err := validatePreExport(ctx, store, jsonlPath); err != nil {
+		t.Errorf("Daemon export scenario failed: validatePreExport blocked after daemon export")
+		t.Errorf("This is the bug from issue #278/#301/#321: %v", err)
+	}
+
+	// Verify we can export again (simulates bd sync)
+	jsonlPathTemp := jsonlPath + ".sync"
+	if err := exportToJSONLWithStore(ctx, store, jsonlPathTemp); err != nil {
+		t.Errorf("Second export (bd sync) failed: %v", err)
+	}
+	os.Remove(jsonlPathTemp)
+}
+
+// TestMultipleExportCycles verifies repeated export cycles don't cause issues
+func TestMultipleExportCycles(t *testing.T) {
+	if testing.Short() {
+		t.Skip("skipping slow test in short mode")
+	}
+
+	tmpDir := t.TempDir()
+	beadsDir := filepath.Join(tmpDir, ".beads")
+	if err := os.Mkdir(beadsDir, 0750); err != nil {
+		t.Fatal(err)
+	}
+
+	dbPath := filepath.Join(beadsDir, "beads.db")
+	jsonlPath := filepath.Join(beadsDir, "issues.jsonl")
+
+	// Create and populate database
+	store, err := sqlite.New(dbPath)
+	if err != nil {
+		t.Fatalf("Failed to create store: %v", err)
+	}
+	defer store.Close()
+
+	ctx := context.Background()
+
+	// Initialize database with issue_prefix
+	if err := store.SetConfig(ctx, "issue_prefix", "test"); err != nil {
+		t.Fatalf("Failed to set issue_prefix: %v", err)
+	}
+
+	// Run multiple export cycles
+	for i := 0; i < 5; i++ {
+		// Add an issue
+		issue := &types.Issue{
+			ID:        "test-" + string(rune('a'+i)),
+			Title:     "Test Issue " + string(rune('A'+i)),
+			Status:    types.StatusOpen,
+			Priority:  2,
+			IssueType: types.TypeTask,
+		}
+		if err := store.CreateIssue(ctx, issue, "test-actor"); err != nil {
+			t.Fatalf("Cycle %d: Failed to create issue: %v", i, err)
+		}
+
+		time.Sleep(100 * time.Millisecond)
+
+		// Export (with fix)
+		if err := exportToJSONLWithStore(ctx, store, jsonlPath); err != nil {
+			t.Fatalf("Cycle %d: Export failed: %v", i, err)
+		}
+
+		// Apply fix
+		if err := TouchDatabaseFile(dbPath, jsonlPath); err != nil {
+			t.Fatalf("Cycle %d: TouchDatabaseFile failed: %v", i, err)
+		}
+
+		// Verify validation passes
+		if err := validatePreExport(ctx, store, jsonlPath); err != nil {
+			t.Errorf("Cycle %d: validatePreExport failed: %v", i, err)
+		}
+	}
+}
--- a/cmd/bd/import.go
+++ b/cmd/bd/import.go
@@ -314,7 +314,7 @@ NOTE: Import requires direct database access and does not work with daemon mode.
 		// 2. Without mtime update, bd sync refuses to export (thinks JSONL is newer)
 		// 3. This can happen after git pull updates JSONL mtime but content is identical
 		// Fix for: refusing to export: JSONL is newer than database (import first to avoid data loss)
-		if err := touchDatabaseFile(dbPath, input); err != nil {
+		if err := TouchDatabaseFile(dbPath, input); err != nil {
 			debug.Logf("Warning: failed to update database mtime: %v", err)
 		}

@@ -381,17 +381,19 @@ NOTE: Import requires direct database access and does not work with daemon mode.
 	},
 }

-// touchDatabaseFile updates the modification time of the database file.
-// This is used after import to ensure the database appears "in sync" with JSONL,
-// preventing bd doctor from incorrectly warning that JSONL is newer.
+// TouchDatabaseFile updates the modification time of the database file.
+// This is used after import AND export to ensure the database appears "in sync" with JSONL,
+// preventing bd doctor and validatePreExport from incorrectly warning that JSONL is newer.
 //
 // In SQLite WAL mode, writes go to beads.db-wal and beads.db mtime may not update
-// until a checkpoint. Since bd doctor compares JSONL mtime to beads.db mtime only,
-// we need to explicitly touch the DB file after import.
+// until a checkpoint. Since validation compares JSONL mtime to beads.db mtime only,
+// we need to explicitly touch the DB file after both import and export operations.
 //
 // The function sets DB mtime to max(JSONL mtime, now) + 1ns to handle clock skew.
 // If jsonlPath is empty or can't be read, falls back to time.Now().
-func touchDatabaseFile(dbPath, jsonlPath string) error {
+//
+// Fixes issues #278, #301, #321: daemon export leaving JSONL newer than DB.
+func TouchDatabaseFile(dbPath, jsonlPath string) error {
 	targetTime := time.Now()
 	
 	// If we have the JSONL path, use max(JSONL mtime, now) to handle clock skew
--- a/cmd/bd/import_mtime_test.go
+++ b/cmd/bd/import_mtime_test.go
@@ -7,7 +7,7 @@ import (
 	"time"
 )

-// TestTouchDatabaseFile verifies the touchDatabaseFile helper function
+// TestTouchDatabaseFile verifies the TouchDatabaseFile helper function
 func TestTouchDatabaseFile(t *testing.T) {
 	tmpDir := t.TempDir()
 	testFile := filepath.Join(tmpDir, "test.db")
@@ -27,8 +27,8 @@ func TestTouchDatabaseFile(t *testing.T) {
 	time.Sleep(1 * time.Second)

 	// Touch the file
-	if err := touchDatabaseFile(testFile, ""); err != nil {
-		t.Fatalf("touchDatabaseFile failed: %v", err)
+	if err := TouchDatabaseFile(testFile, ""); err != nil {
+		t.Fatalf("TouchDatabaseFile failed: %v", err)
 	}

 	// Get new mtime
@@ -64,8 +64,8 @@ func TestTouchDatabaseFileWithClockSkew(t *testing.T) {
 	}

 	// Touch the DB file with JSONL path
-	if err := touchDatabaseFile(dbFile, jsonlFile); err != nil {
-		t.Fatalf("touchDatabaseFile failed: %v", err)
+	if err := TouchDatabaseFile(dbFile, jsonlFile); err != nil {
+		t.Fatalf("TouchDatabaseFile failed: %v", err)
 	}

 	// Get DB mtime
--- a/cmd/bd/sync.go
+++ b/cmd/bd/sync.go
@@ -590,6 +590,15 @@ func exportToJSONL(ctx context.Context, jsonlPath string) error {
 	// Clear auto-flush state
 	clearAutoFlushState()

+	// Update database mtime to be >= JSONL mtime (fixes #278, #301, #321)
+	// This prevents validatePreExport from incorrectly blocking on next export
+	beadsDir := filepath.Dir(jsonlPath)
+	dbPath := filepath.Join(beadsDir, "beads.db")
+	if err := TouchDatabaseFile(dbPath, jsonlPath); err != nil {
+		// Non-fatal warning
+		fmt.Fprintf(os.Stderr, "Warning: failed to update database mtime: %v\n", err)
+	}
+
 	return nil
 }