fix(bd-f2f): Add hash-based staleness detection to prevent stale DB from corrupting JSONL
The existing ZFC checks only compared issue counts, missing the case where counts match but content differs (e.g., status=open vs status=closed). Added Case 3 (bd-f2f) hash-based staleness detection: - Before export, check if JSONL content hash differs from stored hash - If hash mismatch detected, import JSONL first to get remote changes - Then proceed with export to write merged state This prevents the corruption scenario where: 1. Stale DB has old status values (e.g., status=closed) 2. Remote JSONL has correct values (e.g., status=open) 3. Export would overwrite correct JSONL with stale DB values 4. Git 3-way merge would propagate the corruption 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <noreply@anthropic.com>
This commit is contained in:
@@ -223,6 +223,33 @@ Use --merge to merge the sync branch back to main branch.`,
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
// Case 3 (bd-f2f): JSONL content differs from DB (hash mismatch)
|
||||
// This catches the case where counts match but STATUS/content differs.
|
||||
// A stale DB exporting wrong status values over correct JSONL values
|
||||
// causes corruption that the 3-way merge propagates.
|
||||
//
|
||||
// Example: Remote has status=open, stale DB has status=closed (count=5 both)
|
||||
// Without this check: export writes status=closed → git merge keeps it → corruption
|
||||
// With this check: detect hash mismatch → import first → get correct status
|
||||
//
|
||||
// Note: Auto-import in autoflush.go also checks for hash changes during store
|
||||
// initialization, so this check may be redundant in most cases. However, it
|
||||
// provides defense-in-depth for cases where auto-import is disabled or bypassed.
|
||||
if !skipExport {
|
||||
repoKey := getRepoKeyForPath(jsonlPath)
|
||||
if hasJSONLChanged(ctx, store, jsonlPath, repoKey) {
|
||||
fmt.Println("→ JSONL content differs from last sync (bd-f2f)")
|
||||
fmt.Println("→ Importing JSONL first to prevent stale DB from overwriting changes...")
|
||||
if err := importFromJSONL(ctx, jsonlPath, renameOnImport, noGitHistory); err != nil {
|
||||
fmt.Fprintf(os.Stderr, "Error importing (bd-f2f hash mismatch): %v\n", err)
|
||||
os.Exit(1)
|
||||
}
|
||||
// Don't skip export - we still want to export any remaining local dirty issues
|
||||
// The import updated DB with JSONL content, and export will write merged state
|
||||
fmt.Println("→ Import complete, continuing with export of merged state")
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
if !skipExport {
|
||||
|
||||
@@ -1003,3 +1003,101 @@ func TestSanitizeJSONLWithDeletions_NonexistentJSONL(t *testing.T) {
|
||||
t.Errorf("expected 0 removed for missing file, got %d", result.RemovedCount)
|
||||
}
|
||||
}
|
||||
|
||||
// TestHashBasedStalenessDetection_bd_f2f tests the bd-f2f fix:
|
||||
// When JSONL content differs from stored hash (e.g., remote changed status),
|
||||
// hasJSONLChanged should detect the mismatch even if counts are equal.
|
||||
func TestHashBasedStalenessDetection_bd_f2f(t *testing.T) {
|
||||
ctx := context.Background()
|
||||
tmpDir := t.TempDir()
|
||||
|
||||
// Create test database
|
||||
beadsDir := filepath.Join(tmpDir, ".beads")
|
||||
if err := os.MkdirAll(beadsDir, 0755); err != nil {
|
||||
t.Fatalf("failed to create beads dir: %v", err)
|
||||
}
|
||||
|
||||
testDBPath := filepath.Join(beadsDir, "beads.db")
|
||||
jsonlPath := filepath.Join(beadsDir, "issues.jsonl")
|
||||
|
||||
// Create store
|
||||
testStore, err := sqlite.New(ctx, testDBPath)
|
||||
if err != nil {
|
||||
t.Fatalf("failed to create store: %v", err)
|
||||
}
|
||||
defer testStore.Close()
|
||||
|
||||
// Initialize issue prefix (required for creating issues)
|
||||
if err := testStore.SetConfig(ctx, "issue_prefix", "test"); err != nil {
|
||||
t.Fatalf("failed to set issue prefix: %v", err)
|
||||
}
|
||||
|
||||
// Create an issue in DB (simulating stale DB with old content)
|
||||
issue := &types.Issue{
|
||||
ID: "test-abc",
|
||||
Title: "Test Issue",
|
||||
Status: types.StatusOpen,
|
||||
Priority: 1, // DB has priority 1
|
||||
IssueType: types.TypeTask,
|
||||
}
|
||||
if err := testStore.CreateIssue(ctx, issue, "test"); err != nil {
|
||||
t.Fatalf("failed to create issue: %v", err)
|
||||
}
|
||||
|
||||
// Create JSONL with same issue but different priority (correct remote state)
|
||||
// This simulates what happens after git pull brings in updated JSONL
|
||||
// (e.g., remote changed priority from 1 to 0)
|
||||
jsonlContent := `{"id":"test-abc","title":"Test Issue","status":"open","priority":0,"type":"task"}
|
||||
`
|
||||
if err := os.WriteFile(jsonlPath, []byte(jsonlContent), 0600); err != nil {
|
||||
t.Fatalf("failed to write JSONL: %v", err)
|
||||
}
|
||||
|
||||
// Store an OLD hash (different from current JSONL)
|
||||
// This simulates the case where JSONL was updated externally (by git pull)
|
||||
// but DB still has old hash from before the pull
|
||||
oldHash := "0000000000000000000000000000000000000000000000000000000000000000"
|
||||
if err := testStore.SetMetadata(ctx, "jsonl_content_hash", oldHash); err != nil {
|
||||
t.Fatalf("failed to set old hash: %v", err)
|
||||
}
|
||||
|
||||
// Verify counts are equal (1 issue in both)
|
||||
dbCount, err := countDBIssuesFast(ctx, testStore)
|
||||
if err != nil {
|
||||
t.Fatalf("failed to count DB issues: %v", err)
|
||||
}
|
||||
jsonlCount, err := countIssuesInJSONL(jsonlPath)
|
||||
if err != nil {
|
||||
t.Fatalf("failed to count JSONL issues: %v", err)
|
||||
}
|
||||
if dbCount != jsonlCount {
|
||||
t.Fatalf("setup error: expected equal counts, got DB=%d, JSONL=%d", dbCount, jsonlCount)
|
||||
}
|
||||
|
||||
// The key test: hasJSONLChanged should detect the hash mismatch
|
||||
// even though counts are equal
|
||||
repoKey := getRepoKeyForPath(jsonlPath)
|
||||
changed := hasJSONLChanged(ctx, testStore, jsonlPath, repoKey)
|
||||
|
||||
if !changed {
|
||||
t.Error("bd-f2f: hasJSONLChanged should return true when JSONL hash differs from stored hash")
|
||||
t.Log("This is the bug scenario: counts match (1 == 1) but content differs (priority=1 vs priority=0)")
|
||||
t.Log("Without the bd-f2f fix, the stale DB would export old content and corrupt the remote")
|
||||
} else {
|
||||
t.Log("✓ bd-f2f fix verified: hash mismatch detected even with equal counts")
|
||||
}
|
||||
|
||||
// Verify that after updating hash, hasJSONLChanged returns false
|
||||
currentHash, err := computeJSONLHash(jsonlPath)
|
||||
if err != nil {
|
||||
t.Fatalf("failed to compute current hash: %v", err)
|
||||
}
|
||||
if err := testStore.SetMetadata(ctx, "jsonl_content_hash", currentHash); err != nil {
|
||||
t.Fatalf("failed to set current hash: %v", err)
|
||||
}
|
||||
|
||||
changedAfterUpdate := hasJSONLChanged(ctx, testStore, jsonlPath, repoKey)
|
||||
if changedAfterUpdate {
|
||||
t.Error("hasJSONLChanged should return false after hash is updated to match JSONL")
|
||||
}
|
||||
}
|
||||
|
||||
Reference in New Issue
Block a user