Merge branch 'main' of https://github.com/steveyegge/beads

cmd/bd/daemon_sync.go

@@ -251,10 +251,11 @@ func sanitizeMetadataKey(key string) string {
 // In multi-repo mode, keySuffix should be the stable repo identifier (e.g., ".", "../frontend").
 //
 // Metadata key format (bd-ar2.12):
-//   - Single-repo mode: "last_import_hash", "last_import_time", "last_import_mtime"
+//   - Single-repo mode: "last_import_hash", "last_import_time"
 //   - Multi-repo mode: "last_import_hash:<repo_key>", "last_import_time:<repo_key>", etc.
 //     where <repo_key> is a stable repo identifier like "." or "../frontend"
 //   - Windows paths: Colons in absolute paths (e.g., C:\...) are replaced with underscores (bd-web8)
+//   - Note: "last_import_mtime" was removed in bd-v0y fix (git doesn't preserve mtime)
 //
 // Transaction boundaries (bd-ar2.6):
 // This function does NOT provide atomicity between JSONL write, metadata updates, and DB mtime.
@@ -279,11 +280,9 @@ func updateExportMetadata(ctx context.Context, store storage.Storage, jsonlPath
 	// Build metadata keys with optional suffix for per-repo tracking
 	hashKey := "last_import_hash"
 	timeKey := "last_import_time"
-	mtimeKey := "last_import_mtime"
 	if keySuffix != "" {
 		hashKey += ":" + keySuffix
 		timeKey += ":" + keySuffix
-		mtimeKey += ":" + keySuffix
 	}
 
 	// Note: Metadata update failures are treated as warnings, not errors (bd-ar2.5).
@@ -300,14 +299,7 @@ func updateExportMetadata(ctx context.Context, store storage.Storage, jsonlPath
 	if err := store.SetMetadata(ctx, timeKey, exportTime); err != nil {
 		log.log("Warning: failed to update %s: %v", timeKey, err)
 	}
-
-	// Store mtime for fast-path optimization
-	if jsonlInfo, statErr := os.Stat(jsonlPath); statErr == nil {
-		mtimeStr := fmt.Sprintf("%d", jsonlInfo.ModTime().Unix())
-		if err := store.SetMetadata(ctx, mtimeKey, mtimeStr); err != nil {
-			log.log("Warning: failed to update %s: %v", mtimeKey, err)
-		}
-	}
+	// Note: mtime tracking removed in bd-v0y fix (git doesn't preserve mtime)
 }
 
 // validateDatabaseFingerprint checks that the database belongs to this repository

cmd/bd/daemon_sync_test.go

@@ -476,24 +476,7 @@ func TestUpdateExportMetadataMultiRepo(t *testing.T) {
 		t.Error("expected global last_import_hash to not be set when using per-repo keys")
 	}
 
-	// Verify mtime metadata was also set per-repo (bd-web8: keys are sanitized)
-	mtime1Key := "last_import_mtime:" + sanitizeMetadataKey(jsonlPath1)
-	mtime1, err := store.GetMetadata(ctx, mtime1Key)
-	if err != nil {
-		t.Fatalf("failed to get %s: %v", mtime1Key, err)
-	}
-	if mtime1 == "" {
-		t.Errorf("expected %s to be set", mtime1Key)
-	}
-
-	mtime2Key := "last_import_mtime:" + sanitizeMetadataKey(jsonlPath2)
-	mtime2, err := store.GetMetadata(ctx, mtime2Key)
-	if err != nil {
-		t.Fatalf("failed to get %s: %v", mtime2Key, err)
-	}
-	if mtime2 == "" {
-		t.Errorf("expected %s to be set", mtime2Key)
-	}
+	// Note: last_import_mtime removed in bd-v0y fix (git doesn't preserve mtime)
 }
 
 // TestExportWithMultiRepoConfigUpdatesAllMetadata verifies that export with multi-repo
@@ -604,14 +587,7 @@ func TestExportWithMultiRepoConfigUpdatesAllMetadata(t *testing.T) {
 		t.Errorf("expected %s to be set after export", primaryTimeKey)
 	}
 
-	primaryMtimeKey := "last_import_mtime:" + sanitizeMetadataKey(primaryDir)
-	primaryMtime, err := store.GetMetadata(ctx, primaryMtimeKey)
-	if err != nil {
-		t.Fatalf("failed to get %s: %v", primaryMtimeKey, err)
-	}
-	if primaryMtime == "" {
-		t.Errorf("expected %s to be set after export", primaryMtimeKey)
-	}
+	// Note: last_import_mtime removed in bd-v0y fix (git doesn't preserve mtime)
 
 	// Verify metadata for additional repo (bd-web8: keys are sanitized)
 	additionalHashKey := "last_import_hash:" + sanitizeMetadataKey(additionalDir)
@@ -632,14 +608,7 @@ func TestExportWithMultiRepoConfigUpdatesAllMetadata(t *testing.T) {
 		t.Errorf("expected %s to be set after export", additionalTimeKey)
 	}
 
-	additionalMtimeKey := "last_import_mtime:" + sanitizeMetadataKey(additionalDir)
-	additionalMtime, err := store.GetMetadata(ctx, additionalMtimeKey)
-	if err != nil {
-		t.Fatalf("failed to get %s: %v", additionalMtimeKey, err)
-	}
-	if additionalMtime == "" {
-		t.Errorf("expected %s to be set after export", additionalMtimeKey)
-	}
+	// Note: last_import_mtime removed in bd-v0y fix (git doesn't preserve mtime)
 
 	// Note: In this test both JSONL files have the same content (all issues),
 	// so hashes will be identical. In real multi-repo mode, ExportToMultiRepo

cmd/bd/import.go

@@ -337,14 +337,7 @@ NOTE: Import requires direct database access and does not work with daemon mode.
 					// Non-fatal warning (see above comment about graceful degradation)
 					debug.Logf("Warning: failed to update last_import_time: %v", err)
 				}
-				// Store mtime for fast-path optimization in hasJSONLChanged (bd-3bg)
-				if jsonlInfo, statErr := os.Stat(input); statErr == nil {
-					mtimeStr := fmt.Sprintf("%d", jsonlInfo.ModTime().Unix())
-					if err := store.SetMetadata(ctx, "last_import_mtime", mtimeStr); err != nil {
-						// Non-fatal warning (see above comment about graceful degradation)
-						debug.Logf("Warning: failed to update last_import_mtime: %v", err)
-					}
-				}
+				// Note: mtime tracking removed in bd-v0y fix (git doesn't preserve mtime)
 			} else {
 				debug.Logf("Warning: failed to read JSONL for hash update: %v", err)
 			}

cmd/bd/integrity.go

@@ -106,32 +106,14 @@ func hasJSONLChanged(ctx context.Context, store storage.Storage, jsonlPath strin
 
 	// Build metadata keys with optional suffix for per-repo tracking (bd-ar2.10, bd-ar2.11)
 	hashKey := "last_import_hash"
-	mtimeKey := "last_import_mtime"
 	if keySuffix != "" {
 		hashKey += ":" + keySuffix
-		mtimeKey += ":" + keySuffix
 	}
 
-	// Fast-path: Check mtime first to avoid expensive hash computation
-	// Get last known mtime from metadata
-	lastMtimeStr, err := store.GetMetadata(ctx, mtimeKey)
-	if err == nil && lastMtimeStr != "" {
-		// We have a previous mtime - check if file mtime changed
-		jsonlInfo, statErr := os.Stat(jsonlPath)
-		if statErr == nil {
-			currentMtime := jsonlInfo.ModTime().Unix()
-			currentMtimeStr := fmt.Sprintf("%d", currentMtime)
-
-			// If mtime unchanged, content definitely unchanged (filesystem guarantee)
-			// Skip expensive hash computation
-			if currentMtimeStr == lastMtimeStr {
-				return false
-			}
-			// Mtime changed - fall through to hash comparison (could be git operation)
-		}
-	}
-
-	// Slow-path: Compute content hash (either mtime changed or no mtime metadata)
+	// Always compute content hash (bd-v0y fix)
+	// Previous mtime-based fast-path was unsafe: git operations (pull, checkout, rebase)
+	// can change file content without updating mtime, causing false negatives.
+	// Hash computation is fast enough for sync operations (~10-50ms even for large DBs).
 	currentHash, err := computeJSONLHash(jsonlPath)
 	if err != nil {
 		// If we can't read JSONL, assume no change (don't auto-import broken files)

cmd/bd/sync.go

@@ -749,14 +749,7 @@ func exportToJSONL(ctx context.Context, jsonlPath string) error {
 			// Non-fatal warning (see above comment about graceful degradation)
 			fmt.Fprintf(os.Stderr, "Warning: failed to update last_import_time: %v\n", err)
 		}
-		// Store mtime for fast-path optimization in hasJSONLChanged (bd-3bg)
-		if jsonlInfo, statErr := os.Stat(jsonlPath); statErr == nil {
-			mtimeStr := fmt.Sprintf("%d", jsonlInfo.ModTime().Unix())
-			if err := store.SetMetadata(ctx, "last_import_mtime", mtimeStr); err != nil {
-				// Non-fatal warning (see above comment about graceful degradation)
-				fmt.Fprintf(os.Stderr, "Warning: failed to update last_import_mtime: %v\n", err)
-			}
-		}
+		// Note: mtime tracking removed in bd-v0y fix (git doesn't preserve mtime)
 	}
 
 	// Update database mtime to be >= JSONL mtime (fixes #278, #301, #321)