fix(sync): preserve tombstones in sanitizeJSONLWithDeletions (bd-kzxd)
The sanitizeJSONLWithDeletions function was incorrectly removing ALL issues whose ID appeared in deletions.jsonl, including tombstones. This caused: 1. Second sync after delete: tombstone removed from JSONL by sanitize 2. Import sees ID in deletions.jsonl but no tombstone in JSONL 3. Import creates new tombstone via convertDeletionToTombstone 4. UNIQUE constraint error: tombstone already exists in DB The fix checks the issue status and only removes non-tombstone issues. Tombstones are the proper representation of deletions and must be preserved. Added test: TestSanitizeJSONLWithDeletions_PreservesTombstones 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
This commit is contained in:
@@ -1597,9 +1597,14 @@ type SanitizeResult struct {
|
|||||||
RemovedIDs []string // IDs that were removed
|
RemovedIDs []string // IDs that were removed
|
||||||
}
|
}
|
||||||
|
|
||||||
// sanitizeJSONLWithDeletions removes any issues from the JSONL file that are
|
// sanitizeJSONLWithDeletions removes non-tombstone issues from the JSONL file
|
||||||
// in the deletions manifest. This prevents zombie resurrection when git's
|
// if they are in the deletions manifest. This prevents zombie resurrection when
|
||||||
// 3-way merge re-adds deleted issues to the JSONL during pull.
|
// git's 3-way merge re-adds deleted issues to the JSONL during pull.
|
||||||
|
//
|
||||||
|
// IMPORTANT (bd-kzxd fix): Tombstones are NOT removed. Tombstones are the proper
|
||||||
|
// representation of deletions in the JSONL format. Removing them would cause
|
||||||
|
// the importer to re-create tombstones from deletions.jsonl, leading to
|
||||||
|
// UNIQUE constraint errors when the tombstone already exists in the database.
|
||||||
//
|
//
|
||||||
// This should be called after git pull but before import.
|
// This should be called after git pull but before import.
|
||||||
func sanitizeJSONLWithDeletions(jsonlPath string) (*SanitizeResult, error) {
|
func sanitizeJSONLWithDeletions(jsonlPath string) (*SanitizeResult, error) {
|
||||||
@@ -1643,10 +1648,10 @@ func sanitizeJSONLWithDeletions(jsonlPath string) (*SanitizeResult, error) {
|
|||||||
continue
|
continue
|
||||||
}
|
}
|
||||||
|
|
||||||
// Quick extraction of ID without full unmarshal
|
// Extract ID and status to check for tombstones
|
||||||
// Look for "id":"..." pattern
|
|
||||||
var issue struct {
|
var issue struct {
|
||||||
ID string `json:"id"`
|
ID string `json:"id"`
|
||||||
|
Status string `json:"status"`
|
||||||
}
|
}
|
||||||
if err := json.Unmarshal(line, &issue); err != nil {
|
if err := json.Unmarshal(line, &issue); err != nil {
|
||||||
// Keep malformed lines (let import handle them)
|
// Keep malformed lines (let import handle them)
|
||||||
@@ -1656,8 +1661,16 @@ func sanitizeJSONLWithDeletions(jsonlPath string) (*SanitizeResult, error) {
|
|||||||
|
|
||||||
// Check if this ID is in deletions manifest
|
// Check if this ID is in deletions manifest
|
||||||
if _, deleted := loadResult.Records[issue.ID]; deleted {
|
if _, deleted := loadResult.Records[issue.ID]; deleted {
|
||||||
result.RemovedCount++
|
// bd-kzxd fix: Keep tombstones! They are the proper representation of deletions.
|
||||||
result.RemovedIDs = append(result.RemovedIDs, issue.ID)
|
// Only remove non-tombstone issues that were resurrected by git merge.
|
||||||
|
if issue.Status == string(types.StatusTombstone) {
|
||||||
|
// Keep the tombstone - it's the authoritative deletion record
|
||||||
|
keptLines = append(keptLines, append([]byte{}, line...))
|
||||||
|
} else {
|
||||||
|
// Remove non-tombstone issue that was resurrected
|
||||||
|
result.RemovedCount++
|
||||||
|
result.RemovedIDs = append(result.RemovedIDs, issue.ID)
|
||||||
|
}
|
||||||
} else {
|
} else {
|
||||||
keptLines = append(keptLines, append([]byte{}, line...))
|
keptLines = append(keptLines, append([]byte{}, line...))
|
||||||
}
|
}
|
||||||
|
|||||||
@@ -1013,6 +1013,81 @@ func TestSanitizeJSONLWithDeletions_NonexistentJSONL(t *testing.T) {
|
|||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
|
// TestSanitizeJSONLWithDeletions_PreservesTombstones tests the bd-kzxd fix:
|
||||||
|
// Tombstones should NOT be removed by sanitize, even if their ID is in deletions.jsonl.
|
||||||
|
// Tombstones ARE the proper representation of deletions. Removing them would cause
|
||||||
|
// the importer to re-create tombstones from deletions.jsonl, leading to UNIQUE
|
||||||
|
// constraint errors when the tombstone already exists in the database.
|
||||||
|
func TestSanitizeJSONLWithDeletions_PreservesTombstones(t *testing.T) {
|
||||||
|
t.Parallel()
|
||||||
|
tmpDir := t.TempDir()
|
||||||
|
beadsDir := filepath.Join(tmpDir, ".beads")
|
||||||
|
os.MkdirAll(beadsDir, 0755)
|
||||||
|
|
||||||
|
jsonlPath := filepath.Join(beadsDir, "beads.jsonl")
|
||||||
|
deletionsPath := filepath.Join(beadsDir, "deletions.jsonl")
|
||||||
|
|
||||||
|
now := time.Now().Format(time.RFC3339)
|
||||||
|
|
||||||
|
// JSONL with:
|
||||||
|
// - bd-1: regular issue (should be kept)
|
||||||
|
// - bd-2: tombstone (should be kept even though it's in deletions.jsonl)
|
||||||
|
// - bd-3: regular issue that's in deletions.jsonl (should be removed)
|
||||||
|
jsonlContent := fmt.Sprintf(`{"id":"bd-1","title":"Issue 1","status":"open"}
|
||||||
|
{"id":"bd-2","title":"(deleted)","status":"tombstone","deleted_at":"%s","deleted_by":"user"}
|
||||||
|
{"id":"bd-3","title":"Issue 3","status":"open"}
|
||||||
|
`, now)
|
||||||
|
os.WriteFile(jsonlPath, []byte(jsonlContent), 0644)
|
||||||
|
|
||||||
|
// Deletions manifest marks bd-2 and bd-3 as deleted
|
||||||
|
deletionsContent := fmt.Sprintf(`{"id":"bd-2","ts":"%s","by":"user","reason":"cleanup"}
|
||||||
|
{"id":"bd-3","ts":"%s","by":"user","reason":"duplicate"}
|
||||||
|
`, now, now)
|
||||||
|
os.WriteFile(deletionsPath, []byte(deletionsContent), 0644)
|
||||||
|
|
||||||
|
result, err := sanitizeJSONLWithDeletions(jsonlPath)
|
||||||
|
if err != nil {
|
||||||
|
t.Fatalf("unexpected error: %v", err)
|
||||||
|
}
|
||||||
|
|
||||||
|
// Only bd-3 should be removed (non-tombstone issue in deletions)
|
||||||
|
// bd-2 should be kept (it's a tombstone)
|
||||||
|
if result.RemovedCount != 1 {
|
||||||
|
t.Errorf("expected 1 removed (only non-tombstone), got %d", result.RemovedCount)
|
||||||
|
}
|
||||||
|
if len(result.RemovedIDs) != 1 || result.RemovedIDs[0] != "bd-3" {
|
||||||
|
t.Errorf("expected only bd-3 to be removed, got %v", result.RemovedIDs)
|
||||||
|
}
|
||||||
|
|
||||||
|
// Verify JSONL content
|
||||||
|
afterContent, _ := os.ReadFile(jsonlPath)
|
||||||
|
afterStr := string(afterContent)
|
||||||
|
|
||||||
|
// bd-1 should still be present (not in deletions)
|
||||||
|
if !strings.Contains(afterStr, `"id":"bd-1"`) {
|
||||||
|
t.Error("JSONL should still contain bd-1")
|
||||||
|
}
|
||||||
|
|
||||||
|
// bd-2 should still be present (tombstone - preserved!)
|
||||||
|
if !strings.Contains(afterStr, `"id":"bd-2"`) {
|
||||||
|
t.Error("JSONL should still contain bd-2 (tombstone should be preserved)")
|
||||||
|
}
|
||||||
|
if !strings.Contains(afterStr, `"status":"tombstone"`) {
|
||||||
|
t.Error("JSONL should contain tombstone status")
|
||||||
|
}
|
||||||
|
|
||||||
|
// bd-3 should be removed (non-tombstone in deletions)
|
||||||
|
if strings.Contains(afterStr, `"id":"bd-3"`) {
|
||||||
|
t.Error("JSONL should NOT contain bd-3 (non-tombstone in deletions)")
|
||||||
|
}
|
||||||
|
|
||||||
|
// Verify we have exactly 2 issues left (bd-1 and bd-2)
|
||||||
|
afterCount, _ := countIssuesInJSONL(jsonlPath)
|
||||||
|
if afterCount != 2 {
|
||||||
|
t.Errorf("expected 2 issues in JSONL after sanitize, got %d", afterCount)
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
// TestHashBasedStalenessDetection_bd_f2f tests the bd-f2f fix:
|
// TestHashBasedStalenessDetection_bd_f2f tests the bd-f2f fix:
|
||||||
// When JSONL content differs from stored hash (e.g., remote changed status),
|
// When JSONL content differs from stored hash (e.g., remote changed status),
|
||||||
// hasJSONLChanged should detect the mismatch even if counts are equal.
|
// hasJSONLChanged should detect the mismatch even if counts are equal.
|
||||||
|
|||||||
Reference in New Issue
Block a user