johno/beads
1
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

fix(sync): protect locally exported issues from sanitization (bd-3ee1)

Browse Source 
The sync sanitize process was incorrectly removing newly created issues
when they happened to have IDs matching entries in the deletions manifest.
This could occur with hash-based IDs when content is similar to previously
deleted issues.

The fix adds protection for issues that were in the left snapshot (local
export before pull). These represent local work and should not be removed
by sanitize, even if they match entries in the deletions manifest.

Changes:
- Load left snapshot in sanitizeJSONLWithDeletions() to build protection set
- Add protection check before removing issues from JSONL
- Add ProtectedCount/ProtectedIDs to SanitizeResult for tracking
- Log protected issues during sync for visibility
- Add comprehensive test coverage for the fix

🤖 Generated with [Claude Code](https://claude.com/claude-code)

Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
This commit is contained in:
Steve Yegge
2025-12-14 00:55:55 -08:00
parent c125dc617d
commit 3a9749279a
2 changed files with 181 additions and 7 deletions
Download Patch File Download Diff File Expand all files Collapse all files

48
cmd/bd/sync.go
View File

@@ -580,10 +580,19 @@ Use --merge to merge the sync branch back to main branch.`,
if err != nil {
fmt.Fprintf(os.Stderr, "Warning: failed to sanitize JSONL: %v\n", err)
// Non-fatal - continue with import
} else if sanitizeResult.RemovedCount > 0 {
fmt.Printf("→ Sanitized JSONL: removed %d deleted issue(s) that were resurrected by git merge\n", sanitizeResult.RemovedCount)
for _, id := range sanitizeResult.RemovedIDs {
fmt.Printf(" - %s\n", id)
} else {
// bd-3ee1 fix: Log protected issues (local work that would have been incorrectly removed)
if sanitizeResult.ProtectedCount > 0 {
fmt.Printf("→ Protected %d locally exported issue(s) from incorrect sanitization (bd-3ee1)\n", sanitizeResult.ProtectedCount)
for _, id := range sanitizeResult.ProtectedIDs {
fmt.Printf(" - %s (in left snapshot)\n", id)
}
}
if sanitizeResult.RemovedCount > 0 {
fmt.Printf("→ Sanitized JSONL: removed %d deleted issue(s) that were resurrected by git merge\n", sanitizeResult.RemovedCount)
for _, id := range sanitizeResult.RemovedIDs {
fmt.Printf(" - %s\n", id)
}
}
}
@@ -1717,8 +1726,10 @@ func maybeAutoCompactDeletions(ctx context.Context, jsonlPath string) error {
// SanitizeResult contains statistics about the JSONL sanitization operation.
type SanitizeResult struct {
RemovedCount int // Number of issues removed from JSONL
RemovedIDs []string // IDs that were removed
RemovedCount int // Number of issues removed from JSONL
RemovedIDs []string // IDs that were removed
ProtectedCount int // Number of issues protected from removal (bd-3ee1)
ProtectedIDs []string // IDs that were protected
}
// sanitizeJSONLWithDeletions removes non-tombstone issues from the JSONL file
@@ -1730,10 +1741,17 @@ type SanitizeResult struct {
// the importer to re-create tombstones from deletions.jsonl, leading to
// UNIQUE constraint errors when the tombstone already exists in the database.
//
// IMPORTANT (bd-3ee1 fix): Issues that were in the left snapshot (local export
// before pull) are protected from removal. This prevents newly created issues
// from being incorrectly removed when they happen to have an ID that matches
// an entry in the deletions manifest (possible with hash-based IDs if content
// is similar to a previously deleted issue).
//
// This should be called after git pull but before import.
func sanitizeJSONLWithDeletions(jsonlPath string) (*SanitizeResult, error) {
result := &SanitizeResult{
RemovedIDs: []string{},
RemovedIDs: []string{},
ProtectedIDs: []string{},
}
// Get deletions manifest path
@@ -1751,6 +1769,16 @@ func sanitizeJSONLWithDeletions(jsonlPath string) (*SanitizeResult, error) {
return result, nil
}
// bd-3ee1 fix: Load left snapshot to protect locally exported issues
// Issues in the left snapshot were exported before pull and represent
// local work that should not be removed by sanitize
sm := NewSnapshotManager(jsonlPath)
_, leftPath := sm.getSnapshotPaths()
protectedIDs := make(map[string]bool)
if leftIDs, err := sm.buildIDSet(leftPath); err == nil && len(leftIDs) > 0 {
protectedIDs = leftIDs
}
// Read current JSONL
f, err := os.Open(jsonlPath) // #nosec G304 - controlled path
if err != nil {
@@ -1790,6 +1818,12 @@ func sanitizeJSONLWithDeletions(jsonlPath string) (*SanitizeResult, error) {
if issue.Status == string(types.StatusTombstone) {
// Keep the tombstone - it's the authoritative deletion record
keptLines = append(keptLines, append([]byte{}, line...))
} else if protectedIDs[issue.ID] {
// bd-3ee1 fix: Issue was in left snapshot (local export before pull)
// This is local work, not a resurrected zombie - protect it!
keptLines = append(keptLines, append([]byte{}, line...))
result.ProtectedCount++
result.ProtectedIDs = append(result.ProtectedIDs, issue.ID)
} else {
// Remove non-tombstone issue that was resurrected
result.RemovedCount++

140
cmd/bd/sync_test.go
View File

@@ -1088,6 +1088,146 @@ func TestSanitizeJSONLWithDeletions_PreservesTombstones(t *testing.T) {
}
}
// TestSanitizeJSONLWithDeletions_ProtectsLeftSnapshot tests the bd-3ee1 fix:
// Issues that are in the left snapshot (local export before pull) should NOT be
// removed by sanitize, even if they have an ID that matches an entry in the
// deletions manifest. This prevents newly created issues from being incorrectly
// removed when they happen to have an ID that matches a previously deleted issue
// (possible with hash-based IDs if content is similar).
func TestSanitizeJSONLWithDeletions_ProtectsLeftSnapshot(t *testing.T) {
t.Parallel()
tmpDir := t.TempDir()
beadsDir := filepath.Join(tmpDir, ".beads")
os.MkdirAll(beadsDir, 0755)
jsonlPath := filepath.Join(beadsDir, "issues.jsonl")
deletionsPath := filepath.Join(beadsDir, "deletions.jsonl")
leftSnapshotPath := filepath.Join(beadsDir, "beads.left.jsonl")
now := time.Now().Format(time.RFC3339)
// JSONL with:
// - bd-1: regular issue (should be kept - not in deletions)
// - bd-2: regular issue in deletions AND in left snapshot (should be PROTECTED)
// - bd-3: regular issue in deletions but NOT in left snapshot (should be removed)
jsonlContent := `{"id":"bd-1","title":"Issue 1","status":"open"}
{"id":"bd-2","title":"Issue 2","status":"open"}
{"id":"bd-3","title":"Issue 3","status":"open"}
`
os.WriteFile(jsonlPath, []byte(jsonlContent), 0644)
// Left snapshot contains bd-1 and bd-2 (local work before pull)
// bd-2 is the issue we're testing protection for
leftSnapshotContent := `{"id":"bd-1","title":"Issue 1","status":"open"}
{"id":"bd-2","title":"Issue 2","status":"open"}
`
os.WriteFile(leftSnapshotPath, []byte(leftSnapshotContent), 0644)
// Deletions manifest marks bd-2 and bd-3 as deleted
// bd-2 is in deletions but should be protected (it's in left snapshot)
// bd-3 is in deletions and should be removed (it's NOT in left snapshot)
deletionsContent := fmt.Sprintf(`{"id":"bd-2","ts":"%s","by":"user","reason":"old deletion with same ID as new issue"}
{"id":"bd-3","ts":"%s","by":"user","reason":"legitimate deletion"}
`, now, now)
os.WriteFile(deletionsPath, []byte(deletionsContent), 0644)
result, err := sanitizeJSONLWithDeletions(jsonlPath)
if err != nil {
t.Fatalf("unexpected error: %v", err)
}
// bd-3 should be removed (in deletions, not in left snapshot)
if result.RemovedCount != 1 {
t.Errorf("expected 1 removed, 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)
}
// bd-2 should be protected (in left snapshot)
if result.ProtectedCount != 1 {
t.Errorf("expected 1 protected, got %d", result.ProtectedCount)
}
if len(result.ProtectedIDs) != 1 || result.ProtectedIDs[0] != "bd-2" {
t.Errorf("expected bd-2 to be protected, got %v", result.ProtectedIDs)
}
// 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 (protected by left snapshot - bd-3ee1 fix!)
if !strings.Contains(afterStr, `"id":"bd-2"`) {
t.Error("JSONL should still contain bd-2 (protected by left snapshot)")
}
// bd-3 should be removed (in deletions, not protected)
if strings.Contains(afterStr, `"id":"bd-3"`) {
t.Error("JSONL should NOT contain bd-3 (in deletions and not in left snapshot)")
}
// 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)
}
}
// TestSanitizeJSONLWithDeletions_NoLeftSnapshot tests that sanitize still works
// correctly when there's no left snapshot (e.g., first sync or snapshot cleanup).
func TestSanitizeJSONLWithDeletions_NoLeftSnapshot(t *testing.T) {
t.Parallel()
tmpDir := t.TempDir()
beadsDir := filepath.Join(tmpDir, ".beads")
os.MkdirAll(beadsDir, 0755)
jsonlPath := filepath.Join(beadsDir, "issues.jsonl")
deletionsPath := filepath.Join(beadsDir, "deletions.jsonl")
// NOTE: No left snapshot file created
now := time.Now().Format(time.RFC3339)
// JSONL with issues
jsonlContent := `{"id":"bd-1","title":"Issue 1","status":"open"}
{"id":"bd-2","title":"Issue 2","status":"open"}
`
os.WriteFile(jsonlPath, []byte(jsonlContent), 0644)
// Deletions manifest marks bd-2 as deleted
deletionsContent := fmt.Sprintf(`{"id":"bd-2","ts":"%s","by":"user","reason":"deleted"}
`, now)
os.WriteFile(deletionsPath, []byte(deletionsContent), 0644)
result, err := sanitizeJSONLWithDeletions(jsonlPath)
if err != nil {
t.Fatalf("unexpected error: %v", err)
}
// Without left snapshot, bd-2 should be removed (no protection available)
if result.RemovedCount != 1 {
t.Errorf("expected 1 removed, got %d", result.RemovedCount)
}
if result.ProtectedCount != 0 {
t.Errorf("expected 0 protected (no left snapshot), got %d", result.ProtectedCount)
}
// Verify JSONL content
afterContent, _ := os.ReadFile(jsonlPath)
afterStr := string(afterContent)
if !strings.Contains(afterStr, `"id":"bd-1"`) {
t.Error("JSONL should still contain bd-1")
}
if strings.Contains(afterStr, `"id":"bd-2"`) {
t.Error("JSONL should NOT contain bd-2 (no left snapshot protection)")
}
}
// 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.