This commit is contained in:
Steve Yegge
2025-11-20 22:02:51 -05:00
6 changed files with 501 additions and 48 deletions

File diff suppressed because one or more lines are too long

View File

@@ -385,26 +385,13 @@ func createAutoImportFunc(ctx context.Context, store storage.Storage, log daemon
log.log("Removed stale lock (%s), proceeding", holder)
}
// Check JSONL modification time to avoid redundant imports
jsonlInfo, err := os.Stat(jsonlPath)
if err != nil {
log.log("Failed to stat JSONL: %v", err)
return
}
// Get database modification time
dbPath := filepath.Join(beadsDir, "beads.db")
dbInfo, err := os.Stat(dbPath)
if err != nil {
log.log("Failed to stat database: %v", err)
return
}
// Skip if JSONL is older than database (nothing new to import)
if !jsonlInfo.ModTime().After(dbInfo.ModTime()) {
log.log("Skipping import: JSONL not newer than database")
// Check JSONL content hash to avoid redundant imports
// Use content-based check (not mtime) to avoid git resurrection bug (bd-khnb)
if !hasJSONLChanged(importCtx, store, jsonlPath) {
log.log("Skipping import: JSONL content unchanged")
return
}
log.log("JSONL content changed, proceeding with import...")
// Pull from git (try sync branch first)
pulled, err := syncBranchPull(importCtx, store, log)

View File

@@ -307,6 +307,34 @@ NOTE: Import requires direct database access and does not work with daemon mode.
flushToJSONL()
}
// Update last_import_hash metadata to enable content-based staleness detection (bd-khnb fix)
// This prevents git operations from resurrecting deleted issues by comparing content instead of mtime
if input != "" {
if currentHash, err := computeJSONLHash(input); err == nil {
if err := store.SetMetadata(ctx, "last_import_hash", currentHash); err != nil {
// Non-fatal warning: Metadata update failures are intentionally non-fatal to prevent blocking
// successful imports. System degrades gracefully to mtime-based staleness detection if metadata
// is unavailable. This ensures import operations always succeed even if metadata storage fails.
debug.Logf("Warning: failed to update last_import_hash: %v", err)
}
importTime := time.Now().Format(time.RFC3339)
if err := store.SetMetadata(ctx, "last_import_time", importTime); err != nil {
// 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)
}
}
} else {
debug.Logf("Warning: failed to read JSONL for hash update: %v", err)
}
}
// Update database mtime to reflect it's now in sync with JSONL
// This is CRITICAL even when import found 0 changes, because:
// 1. Import validates DB and JSONL are in sync (no content divergence)

View File

@@ -20,6 +20,10 @@ import (
// isJSONLNewer checks if JSONL file is newer than database file.
// Returns true if JSONL is newer AND has different content, false otherwise.
// This prevents false positives from daemon auto-export timestamp skew (bd-lm2q).
//
// NOTE: This uses computeDBHash which is more expensive than hasJSONLChanged.
// For daemon auto-import, prefer hasJSONLChanged() which uses metadata-based
// content tracking and is safe against git operations (bd-khnb).
func isJSONLNewer(jsonlPath string) bool {
return isJSONLNewerWithStore(jsonlPath, nil)
}
@@ -71,12 +75,71 @@ func isJSONLNewerWithStore(jsonlPath string, st storage.Storage) bool {
return jsonlHash != dbHash
}
// computeJSONLHash computes SHA256 hash of JSONL file content.
// Returns hex-encoded hash string and any error encountered reading the file.
func computeJSONLHash(jsonlPath string) (string, error) {
jsonlData, err := os.ReadFile(jsonlPath) // #nosec G304 - controlled path
if err != nil {
return "", err
}
hasher := sha256.New()
hasher.Write(jsonlData)
return hex.EncodeToString(hasher.Sum(nil)), nil
}
// hasJSONLChanged checks if JSONL content has changed since last import using SHA256 hash.
// Returns true if JSONL content differs from last import, false otherwise.
// This is safe against git operations that restore old files with recent mtimes.
//
// Performance optimization: Checks mtime first as a fast-path. Only computes expensive
// SHA256 hash if mtime changed. This makes 99% of checks instant (mtime unchanged = content
// unchanged) while still catching git operations that restore old content with new mtimes.
func hasJSONLChanged(ctx context.Context, store storage.Storage, jsonlPath string) bool {
// Fast-path: Check mtime first to avoid expensive hash computation
// Get last known mtime from metadata
lastMtimeStr, err := store.GetMetadata(ctx, "last_import_mtime")
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)
currentHash, err := computeJSONLHash(jsonlPath)
if err != nil {
// If we can't read JSONL, assume no change (don't auto-import broken files)
return false
}
// Get last import hash from metadata
lastHash, err := store.GetMetadata(ctx, "last_import_hash")
if err != nil {
// No previous import hash - this is the first run or metadata is missing
// Assume changed to trigger import
return true
}
// Compare hashes
return currentHash != lastHash
}
// validatePreExport performs integrity checks before exporting database to JSONL.
// Returns error if critical issues found that would cause data loss.
func validatePreExport(ctx context.Context, store storage.Storage, jsonlPath string) error {
// Check if JSONL is newer than database - if so, must import first
if isJSONLNewer(jsonlPath) {
return fmt.Errorf("refusing to export: JSONL is newer than database (import first to avoid data loss)")
// Check if JSONL content has changed since last import - if so, must import first
// Uses content-based detection (bd-xwo fix) instead of mtime-based to avoid false positives from git operations
if hasJSONLChanged(ctx, store, jsonlPath) {
return fmt.Errorf("refusing to export: JSONL content has changed since last import (import first to avoid data loss)")
}
jsonlInfo, jsonlStatErr := os.Stat(jsonlPath)

View File

@@ -2,6 +2,7 @@ package main
import (
"context"
"fmt"
"os"
"path/filepath"
"strings"
@@ -67,8 +68,17 @@ func TestValidatePreExport(t *testing.T) {
t.Fatalf("Failed to write JSONL: %v", err)
}
// Store hash metadata to indicate JSONL and DB are in sync
hash, err := computeJSONLHash(jsonlPath)
if err != nil {
t.Fatalf("Failed to compute hash: %v", err)
}
if err := store.SetMetadata(ctx, "last_import_hash", hash); err != nil {
t.Fatalf("Failed to set hash metadata: %v", err)
}
// Should pass validation
err := validatePreExport(ctx, store, jsonlPath)
err = validatePreExport(ctx, store, jsonlPath)
if err != nil {
t.Errorf("Expected no error, got: %v", err)
}
@@ -114,7 +124,7 @@ func TestValidatePreExport(t *testing.T) {
}
})
t.Run("JSONL newer than DB fails", func(t *testing.T) {
t.Run("JSONL content changed fails", func(t *testing.T) {
// Create temp directory
tmpDir := t.TempDir()
beadsDir := filepath.Join(tmpDir, ".beads")
@@ -139,27 +149,36 @@ func TestValidatePreExport(t *testing.T) {
t.Fatalf("Failed to create issue: %v", err)
}
// Create JSONL file with newer timestamp
// Create initial JSONL file
jsonlContent := `{"id":"bd-1","title":"Test","status":"open","priority":1}
`
if err := os.WriteFile(jsonlPath, []byte(jsonlContent), 0600); err != nil {
t.Fatalf("Failed to write JSONL: %v", err)
}
// Touch JSONL to make it newer than DB
// (in real scenario, this happens when git pull updates JSONL but daemon hasn't imported yet)
futureTime := time.Now().Add(1 * time.Second)
if err := os.Chtimes(jsonlPath, futureTime, futureTime); err != nil {
t.Fatalf("Failed to touch JSONL: %v", err)
// Store hash of original content
hash, err := computeJSONLHash(jsonlPath)
if err != nil {
t.Fatalf("Failed to compute hash: %v", err)
}
if err := store.SetMetadata(ctx, "last_import_hash", hash); err != nil {
t.Fatalf("Failed to set hash: %v", err)
}
// Should fail validation (JSONL is newer, must import first)
err := validatePreExport(ctx, store, jsonlPath)
if err == nil {
t.Error("Expected error for JSONL newer than DB, got nil")
// Modify JSONL content (simulates git pull that changed JSONL)
modifiedContent := `{"id":"bd-1","title":"Modified","status":"open","priority":1}
`
if err := os.WriteFile(jsonlPath, []byte(modifiedContent), 0600); err != nil {
t.Fatalf("Failed to write modified JSONL: %v", err)
}
if err != nil && !strings.Contains(err.Error(), "JSONL is newer than database") {
t.Errorf("Expected 'JSONL is newer' error, got: %v", err)
// Should fail validation (JSONL content changed, must import first)
err = validatePreExport(ctx, store, jsonlPath)
if err == nil {
t.Error("Expected error for changed JSONL content, got nil")
}
if err != nil && !strings.Contains(err.Error(), "JSONL content has changed") {
t.Errorf("Expected 'JSONL content has changed' error, got: %v", err)
}
})
}
@@ -232,6 +251,320 @@ func TestCountDBIssues(t *testing.T) {
})
}
func TestHasJSONLChanged(t *testing.T) {
ctx := context.Background()
t.Run("hash matches - no change", func(t *testing.T) {
tmpDir := t.TempDir()
dbPath := filepath.Join(tmpDir, "test.db")
jsonlPath := filepath.Join(tmpDir, "issues.jsonl")
// Create database
store := newTestStore(t, dbPath)
// Create JSONL file
jsonlContent := `{"id":"bd-1","title":"Test","status":"open","priority":1}
`
if err := os.WriteFile(jsonlPath, []byte(jsonlContent), 0600); err != nil {
t.Fatalf("Failed to write JSONL: %v", err)
}
// Compute hash and store it
hash, err := computeJSONLHash(jsonlPath)
if err != nil {
t.Fatalf("Failed to compute hash: %v", err)
}
if err := store.SetMetadata(ctx, "last_import_hash", hash); err != nil {
t.Fatalf("Failed to set metadata: %v", err)
}
// Store mtime for fast-path
if info, err := os.Stat(jsonlPath); err == nil {
mtimeStr := fmt.Sprintf("%d", info.ModTime().Unix())
if err := store.SetMetadata(ctx, "last_import_mtime", mtimeStr); err != nil {
t.Fatalf("Failed to set mtime: %v", err)
}
}
// Should return false (no change)
if hasJSONLChanged(ctx, store, jsonlPath) {
t.Error("Expected hasJSONLChanged to return false for matching hash")
}
})
t.Run("hash differs - has changed", func(t *testing.T) {
tmpDir := t.TempDir()
dbPath := filepath.Join(tmpDir, "test.db")
jsonlPath := filepath.Join(tmpDir, "issues.jsonl")
// Create database
store := newTestStore(t, dbPath)
// Create initial JSONL file
jsonlContent := `{"id":"bd-1","title":"Test","status":"open","priority":1}
`
if err := os.WriteFile(jsonlPath, []byte(jsonlContent), 0600); err != nil {
t.Fatalf("Failed to write JSONL: %v", err)
}
// Compute hash and store it
hash, err := computeJSONLHash(jsonlPath)
if err != nil {
t.Fatalf("Failed to compute hash: %v", err)
}
if err := store.SetMetadata(ctx, "last_import_hash", hash); err != nil {
t.Fatalf("Failed to set metadata: %v", err)
}
// Modify JSONL file
newContent := `{"id":"bd-1","title":"Modified","status":"open","priority":1}
`
if err := os.WriteFile(jsonlPath, []byte(newContent), 0600); err != nil {
t.Fatalf("Failed to write modified JSONL: %v", err)
}
// Should return true (content changed)
if !hasJSONLChanged(ctx, store, jsonlPath) {
t.Error("Expected hasJSONLChanged to return true for different hash")
}
})
t.Run("empty file", func(t *testing.T) {
tmpDir := t.TempDir()
dbPath := filepath.Join(tmpDir, "test.db")
jsonlPath := filepath.Join(tmpDir, "issues.jsonl")
// Create database
store := newTestStore(t, dbPath)
// Create empty JSONL file
if err := os.WriteFile(jsonlPath, []byte(""), 0600); err != nil {
t.Fatalf("Failed to write empty JSONL: %v", err)
}
// Should return true (no previous hash, first run)
if !hasJSONLChanged(ctx, store, jsonlPath) {
t.Error("Expected hasJSONLChanged to return true for empty file with no metadata")
}
})
t.Run("missing metadata - first run", func(t *testing.T) {
tmpDir := t.TempDir()
dbPath := filepath.Join(tmpDir, "test.db")
jsonlPath := filepath.Join(tmpDir, "issues.jsonl")
// Create database
store := newTestStore(t, dbPath)
// Create JSONL file
jsonlContent := `{"id":"bd-1","title":"Test","status":"open","priority":1}
`
if err := os.WriteFile(jsonlPath, []byte(jsonlContent), 0600); err != nil {
t.Fatalf("Failed to write JSONL: %v", err)
}
// No metadata stored - should return true (assume changed)
if !hasJSONLChanged(ctx, store, jsonlPath) {
t.Error("Expected hasJSONLChanged to return true when no metadata exists")
}
})
t.Run("file read error", func(t *testing.T) {
tmpDir := t.TempDir()
dbPath := filepath.Join(tmpDir, "test.db")
jsonlPath := filepath.Join(tmpDir, "nonexistent.jsonl")
// Create database
store := newTestStore(t, dbPath)
// File doesn't exist - should return false (don't auto-import broken files)
if hasJSONLChanged(ctx, store, jsonlPath) {
t.Error("Expected hasJSONLChanged to return false for nonexistent file")
}
})
t.Run("mtime fast-path - unchanged", func(t *testing.T) {
tmpDir := t.TempDir()
dbPath := filepath.Join(tmpDir, "test.db")
jsonlPath := filepath.Join(tmpDir, "issues.jsonl")
// Create database
store := newTestStore(t, dbPath)
// Create JSONL file
jsonlContent := `{"id":"bd-1","title":"Test","status":"open","priority":1}
`
if err := os.WriteFile(jsonlPath, []byte(jsonlContent), 0600); err != nil {
t.Fatalf("Failed to write JSONL: %v", err)
}
// Get file info
info, err := os.Stat(jsonlPath)
if err != nil {
t.Fatalf("Failed to stat JSONL: %v", err)
}
// Store hash and mtime
hash, err := computeJSONLHash(jsonlPath)
if err != nil {
t.Fatalf("Failed to compute hash: %v", err)
}
if err := store.SetMetadata(ctx, "last_import_hash", hash); err != nil {
t.Fatalf("Failed to set hash: %v", err)
}
mtimeStr := fmt.Sprintf("%d", info.ModTime().Unix())
if err := store.SetMetadata(ctx, "last_import_mtime", mtimeStr); err != nil {
t.Fatalf("Failed to set mtime: %v", err)
}
// Should return false using fast-path (mtime unchanged)
if hasJSONLChanged(ctx, store, jsonlPath) {
t.Error("Expected hasJSONLChanged to return false using mtime fast-path")
}
})
t.Run("mtime changed but content same - git operation scenario", func(t *testing.T) {
tmpDir := t.TempDir()
dbPath := filepath.Join(tmpDir, "test.db")
jsonlPath := filepath.Join(tmpDir, "issues.jsonl")
// Create database
store := newTestStore(t, dbPath)
// Create JSONL file
jsonlContent := `{"id":"bd-1","title":"Test","status":"open","priority":1}
`
if err := os.WriteFile(jsonlPath, []byte(jsonlContent), 0600); err != nil {
t.Fatalf("Failed to write JSONL: %v", err)
}
// Get initial file info
initialInfo, err := os.Stat(jsonlPath)
if err != nil {
t.Fatalf("Failed to stat JSONL: %v", err)
}
// Store hash and old mtime
hash, err := computeJSONLHash(jsonlPath)
if err != nil {
t.Fatalf("Failed to compute hash: %v", err)
}
if err := store.SetMetadata(ctx, "last_import_hash", hash); err != nil {
t.Fatalf("Failed to set hash: %v", err)
}
oldMtime := fmt.Sprintf("%d", initialInfo.ModTime().Unix()-1000) // Old mtime
if err := store.SetMetadata(ctx, "last_import_mtime", oldMtime); err != nil {
t.Fatalf("Failed to set old mtime: %v", err)
}
// Touch file to simulate git operation (new mtime, same content)
time.Sleep(10 * time.Millisecond) // Ensure time passes
futureTime := time.Now().Add(1 * time.Second)
if err := os.Chtimes(jsonlPath, futureTime, futureTime); err != nil {
t.Fatalf("Failed to touch JSONL: %v", err)
}
// Should return false (content hasn't changed despite new mtime)
if hasJSONLChanged(ctx, store, jsonlPath) {
t.Error("Expected hasJSONLChanged to return false for git operation with same content")
}
})
}
func TestComputeJSONLHash(t *testing.T) {
t.Run("computes hash correctly", func(t *testing.T) {
tmpDir := t.TempDir()
jsonlPath := filepath.Join(tmpDir, "issues.jsonl")
jsonlContent := `{"id":"bd-1","title":"Test","status":"open","priority":1}
`
if err := os.WriteFile(jsonlPath, []byte(jsonlContent), 0600); err != nil {
t.Fatalf("Failed to write JSONL: %v", err)
}
hash, err := computeJSONLHash(jsonlPath)
if err != nil {
t.Fatalf("Failed to compute hash: %v", err)
}
if hash == "" {
t.Error("Expected non-empty hash")
}
if len(hash) != 64 { // SHA256 hex is 64 chars
t.Errorf("Expected hash length 64, got %d", len(hash))
}
})
t.Run("same content produces same hash", func(t *testing.T) {
tmpDir := t.TempDir()
jsonlPath1 := filepath.Join(tmpDir, "issues1.jsonl")
jsonlPath2 := filepath.Join(tmpDir, "issues2.jsonl")
jsonlContent := `{"id":"bd-1","title":"Test","status":"open","priority":1}
`
if err := os.WriteFile(jsonlPath1, []byte(jsonlContent), 0600); err != nil {
t.Fatalf("Failed to write JSONL 1: %v", err)
}
if err := os.WriteFile(jsonlPath2, []byte(jsonlContent), 0600); err != nil {
t.Fatalf("Failed to write JSONL 2: %v", err)
}
hash1, err := computeJSONLHash(jsonlPath1)
if err != nil {
t.Fatalf("Failed to compute hash 1: %v", err)
}
hash2, err := computeJSONLHash(jsonlPath2)
if err != nil {
t.Fatalf("Failed to compute hash 2: %v", err)
}
if hash1 != hash2 {
t.Errorf("Expected same hash for same content, got %s and %s", hash1, hash2)
}
})
t.Run("different content produces different hash", func(t *testing.T) {
tmpDir := t.TempDir()
jsonlPath1 := filepath.Join(tmpDir, "issues1.jsonl")
jsonlPath2 := filepath.Join(tmpDir, "issues2.jsonl")
if err := os.WriteFile(jsonlPath1, []byte(`{"id":"bd-1"}`), 0600); err != nil {
t.Fatalf("Failed to write JSONL 1: %v", err)
}
if err := os.WriteFile(jsonlPath2, []byte(`{"id":"bd-2"}`), 0600); err != nil {
t.Fatalf("Failed to write JSONL 2: %v", err)
}
hash1, err := computeJSONLHash(jsonlPath1)
if err != nil {
t.Fatalf("Failed to compute hash 1: %v", err)
}
hash2, err := computeJSONLHash(jsonlPath2)
if err != nil {
t.Fatalf("Failed to compute hash 2: %v", err)
}
if hash1 == hash2 {
t.Errorf("Expected different hashes for different content")
}
})
t.Run("nonexistent file returns error", func(t *testing.T) {
tmpDir := t.TempDir()
jsonlPath := filepath.Join(tmpDir, "nonexistent.jsonl")
_, err := computeJSONLHash(jsonlPath)
if err == nil {
t.Error("Expected error for nonexistent file, got nil")
}
})
}
func TestCheckOrphanedDeps(t *testing.T) {
t.Run("function executes without error", func(t *testing.T) {
// Create temp directory

View File

@@ -126,14 +126,17 @@ Use --merge to merge the sync branch back to main branch.`,
if dryRun {
fmt.Println("→ [DRY RUN] Would export pending changes to JSONL")
} else {
// Smart conflict resolution: if JSONL is newer, auto-import first
if isJSONLNewer(jsonlPath) {
fmt.Println("→ JSONL is newer than database, importing first...")
if err := importFromJSONL(ctx, jsonlPath, renameOnImport); err != nil {
fmt.Fprintf(os.Stderr, "Error auto-importing: %v\n", err)
os.Exit(1)
// Smart conflict resolution: if JSONL content changed, auto-import first
// Use content-based check (not mtime) to avoid git resurrection bug (bd-khnb)
if err := ensureStoreActive(); err == nil && store != nil {
if hasJSONLChanged(ctx, store, jsonlPath) {
fmt.Println("→ JSONL content changed, importing first...")
if err := importFromJSONL(ctx, jsonlPath, renameOnImport); err != nil {
fmt.Fprintf(os.Stderr, "Error auto-importing: %v\n", err)
os.Exit(1)
}
fmt.Println("✓ Auto-import complete")
}
fmt.Println("✓ Auto-import complete")
}
// Pre-export integrity checks
@@ -600,6 +603,30 @@ func exportToJSONL(ctx context.Context, jsonlPath string) error {
// Clear auto-flush state
clearAutoFlushState()
// Update last_import_hash metadata to enable content-based staleness detection (bd-khnb fix)
// After export, database and JSONL are in sync, so update hash to prevent unnecessary auto-import
if currentHash, err := computeJSONLHash(jsonlPath); err == nil {
if err := store.SetMetadata(ctx, "last_import_hash", currentHash); err != nil {
// Non-fatal warning: Metadata update failures are intentionally non-fatal to prevent blocking
// successful exports. System degrades gracefully to mtime-based staleness detection if metadata
// is unavailable. This ensures export operations always succeed even if metadata storage fails.
fmt.Fprintf(os.Stderr, "Warning: failed to update last_import_hash: %v\n", err)
}
exportTime := time.Now().Format(time.RFC3339)
if err := store.SetMetadata(ctx, "last_import_time", exportTime); err != nil {
// 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)
}
}
}
// Update database mtime to be >= JSONL mtime (fixes #278, #301, #321)
// This prevents validatePreExport from incorrectly blocking on next export
beadsDir := filepath.Dir(jsonlPath)