diff --git a/cmd/bd/daemon_sync.go b/cmd/bd/daemon_sync.go index 7815c828..0999138c 100644 --- 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 { diff --git a/cmd/bd/export.go b/cmd/bd/export.go index be194229..d5bba35a 100644 --- 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 diff --git a/cmd/bd/export_mtime_test.go b/cmd/bd/export_mtime_test.go new file mode 100644 index 00000000..42db5d88 --- /dev/null +++ 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) + } + } +} diff --git a/cmd/bd/import.go b/cmd/bd/import.go index 1d832da9..4f02d8cc 100644 --- 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 diff --git a/cmd/bd/import_mtime_test.go b/cmd/bd/import_mtime_test.go index 51db5720..13b5e461 100644 --- 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 diff --git a/cmd/bd/sync.go b/cmd/bd/sync.go index b9bd0818..bbb751d4 100644 --- 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 }