Fix race condition in dirty issue tracking (bd-52, bd-53)

Fix critical TOCTOU bug where concurrent operations could lose dirty
issue tracking, causing data loss in incremental exports. Also fixes
bug where export with filters would incorrectly clear all dirty issues.

The Problem:
1. GetDirtyIssues() returns [bd-1, bd-2]
2. Concurrent CRUD marks bd-3 dirty
3. Export writes bd-1, bd-2
4. ClearDirtyIssues() deletes ALL (including bd-3)
5. Result: bd-3 never gets exported!

The Fix:
- Add ClearDirtyIssuesByID() that only clears specific issue IDs
- Track which issues were actually exported
- Clear only those specific IDs, not all dirty issues
- Fixes both race condition and filter export bug

Changes:
- internal/storage/sqlite/dirty.go:
  * Add ClearDirtyIssuesByID() method
  * Add warning to ClearDirtyIssues() about race condition
- internal/storage/storage.go:
  * Add ClearDirtyIssuesByID to interface
- cmd/bd/main.go:
  * Update auto-flush to use ClearDirtyIssuesByID()
- cmd/bd/export.go:
  * Track exported issue IDs
  * Use ClearDirtyIssuesByID() instead of ClearDirtyIssues()

Testing:
- Created test-1, test-2, test-3 (all dirty)
- Updated test-2 to in_progress
- Exported with --status open filter (exports only test-1, test-3)
- Verified only test-2 remains dirty ✓
- All existing tests pass ✓

Impact:
- Race condition eliminated - concurrent operations are safe
- Export with filters now works correctly
- No data loss from competing writes

Closes bd-52, bd-53

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

Co-Authored-By: Claude <noreply@anthropic.com>
This commit is contained in:
Steve Yegge
2025-10-14 00:29:23 -07:00
parent f3a61a6458
commit 92759710de
4 changed files with 39 additions and 5 deletions

View File

@@ -76,18 +76,20 @@ Output to stdout by default, or use -o flag for file output.`,
// Write JSONL
encoder := json.NewEncoder(out)
exportedIDs := make([]string, 0, len(issues))
for _, issue := range issues {
if err := encoder.Encode(issue); err != nil {
fmt.Fprintf(os.Stderr, "Error encoding issue %s: %v\n", issue.ID, err)
os.Exit(1)
}
exportedIDs = append(exportedIDs, issue.ID)
}
// Only clear dirty issues and auto-flush state if exporting to the default JSONL path
// This prevents clearing dirty flags when exporting to custom paths (e.g., bd export -o backup.jsonl)
if output == "" || output == findJSONLPath() {
// Clear dirty issues since we just exported to the canonical JSONL file
if err := store.ClearDirtyIssues(ctx); err != nil {
// Clear only the issues that were actually exported (fixes bd-52 race condition)
if err := store.ClearDirtyIssuesByID(ctx, exportedIDs); err != nil {
fmt.Fprintf(os.Stderr, "Warning: failed to clear dirty issues: %v\n", err)
}

View File

@@ -516,8 +516,8 @@ func flushToJSONL() {
return
}
// Clear dirty issues after successful export
if err := store.ClearDirtyIssues(ctx); err != nil {
// Clear only the dirty issues that were actually exported (fixes bd-52 race condition)
if err := store.ClearDirtyIssuesByID(ctx, dirtyIDs); err != nil {
// Don't fail the whole flush for this, but warn
fmt.Fprintf(os.Stderr, "Warning: failed to clear dirty issues: %v\n", err)
}