diff --git a/.beads/beads.jsonl b/.beads/beads.jsonl index 7d16768d..fd516aee 100644 --- a/.beads/beads.jsonl +++ b/.beads/beads.jsonl @@ -5,7 +5,7 @@ {"id":"bd-1qwo","title":"Audit and enforce consistent error handling patterns across codebase","description":"**Background:** Error handling patterns are now documented in docs/ERROR_HANDLING.md (bd-9lwr). We have three established patterns:\n- Pattern A: Exit immediately (os.Exit) for fatal errors\n- Pattern B: Warn and continue for optional operations\n- Pattern C: Silent ignore for cleanup/best-effort\n\n**Goal:** Systematically review all error handling in cmd/bd to ensure each instance uses the appropriate pattern according to the guidelines.\n\n**Scope:** \n- Review all files in cmd/bd/*.go\n- Focus on files with high usage: create.go, init.go, sync.go, daemon_sync.go, export.go, import.go\n- Identify outliers where similar operations use different patterns\n- Refactor to use consistent patterns\n\n**Acceptance Criteria:**\n- Similar operations (e.g., all metadata updates) use the same pattern\n- Critical operations always use Pattern A\n- Auxiliary operations use Pattern B or C appropriately\n- No mixed patterns for identical operation types\n\n**References:**\n- docs/ERROR_HANDLING.md - Guidelines and decision tree\n- bd-9lwr - Documentation task\n- bd-bwk2 - Related storage layer error handling work","status":"closed","priority":2,"issue_type":"task","created_at":"2025-11-23T21:53:25.687087-08:00","updated_at":"2025-11-24T01:08:18.294375-08:00","closed_at":"2025-11-24T00:28:58.356196-08:00"} {"id":"bd-1rh","title":"cmd/bd test suite is absurdly slow - 279 tests taking 8+ minutes","description":"# Problem\n\nThe cmd/bd test suite is painfully slow:\n- **279 tests** in cmd/bd alone\n- Full suite takes **8+ minutes** to run\n- Even with the 16 slowest integration tests now tagged with `integration` build tag, the remaining tests still take forever\n\nThis makes the development loop unusable. We can't wait 8+ minutes every time we want to run tests.\n\n# Root Cause Analysis\n\n## 1. Sheer Volume\n279 tests is too many for a single package. Even at 0.1s per test, that's 28 seconds minimum just for cmd/bd.\n\n## 2. Each Test Creates Full Database + Temp Directories\nEvery test does heavy setup:\n- Creates temp directory (`t.TempDir()` or `os.MkdirTemp`)\n- Initializes SQLite database\n- Sets up git repo in many cases\n- Creates full storage layer\n\nExample from the tests:\n```go\nfunc setupCLITestDB(t *testing.T) string {\n tmpDir := createTempDirWithCleanup(t)\n runBDInProcess(t, tmpDir, \"init\", \"--prefix\", \"test\", \"--quiet\")\n return tmpDir\n}\n```\n\nThis happens 279 times!\n\n## 3. Tests Are Not Properly Categorized\nWe have three types of tests mixed together:\n- **Unit tests** - should be fast, test single functions\n- **Integration tests** - test full workflows, need DB/git\n- **End-to-end tests** - test entire CLI commands\n\nThey're all lumped together in cmd/bd, all running every time.\n\n# What We've Already Fixed\n\nAdded `integration` build tags to 16 obviously-slow test files:\n- import_profile_test.go (performance benchmarking tests)\n- export_mtime_test.go (tests with time.Sleep calls)\n- cli_fast_test.go (full CLI integration tests)\n- delete_test.go, import_uncommitted_test.go, sync_local_only_test.go (git integration)\n- And 10 more in internal/ packages\n\nThese are now excluded from the default `go test ./...` run.\n\n# Proposed Solutions\n\n## Option 1: Shared Test Fixtures (Quick Win)\nCreate a shared test database that multiple tests can use:\n```go\nvar testDB *sqlite.SQLiteStorage\nvar testDBOnce sync.Once\n\nfunc getSharedTestDB(t *testing.T) storage.Storage {\n testDBOnce.Do(func() {\n // Create one DB for all tests\n })\n return testDB\n}\n```\n\n**Pros**: Easy to implement, immediate speedup\n**Cons**: Tests become less isolated, harder to debug failures\n\n## Option 2: Table-Driven Tests (Medium Win)\nCollapse similar tests into table-driven tests:\n```go\nfunc TestCreate(t *testing.T) {\n tests := []struct{\n name string\n args []string\n want string\n }{\n {\"basic issue\", []string{\"create\", \"Test\"}, \"created\"},\n {\"with description\", []string{\"create\", \"Test\", \"-d\", \"desc\"}, \"created\"},\n // ... 50 more cases\n }\n \n db := setupOnce(t) // Setup once, not 50 times\n for _, tt := range tests {\n t.Run(tt.name, func(t *testing.T) {\n // test using shared db\n })\n }\n}\n```\n\n**Pros**: Dramatically reduces setup overhead, tests run in parallel\n**Cons**: Requires refactoring, tests share more state\n\n## Option 3: Split cmd/bd Tests Into Packages (Big Win)\nMove tests into focused packages:\n- `cmd/bd/internal/clitests` - CLI integration tests (mark with integration tag)\n- `cmd/bd/internal/unittests` - Fast unit tests\n- Keep only essential tests in cmd/bd\n\n**Pros**: Clean separation, easy to run just fast tests\n**Cons**: Requires significant refactoring\n\n## Option 4: Parallel Execution (Quick Win)\nAdd `t.Parallel()` to independent tests:\n```go\nfunc TestSomething(t *testing.T) {\n t.Parallel() // Run this test concurrently with others\n // ...\n}\n```\n\n**Pros**: Easy to add, can cut time in half on multi-core machines\n**Cons**: Doesn't reduce actual test work, just parallelizes it\n\n## Option 5: In-Memory Databases (Medium Win)\nUse `:memory:` SQLite databases instead of file-based:\n```go\nstore, err := sqlite.New(ctx, \":memory:\")\n```\n\n**Pros**: Faster than disk I/O, easier cleanup\n**Cons**: Some tests need actual file-based DBs (export/import tests)\n\n# Recommended Approach\n\n**Short-term (this week)**:\n1. Add `t.Parallel()` to all independent tests in cmd/bd\n2. Use `:memory:` databases where possible\n3. Create table-driven tests for similar test cases\n\n**Medium-term (next sprint)**:\n4. Split cmd/bd tests into focused packages\n5. Mark more integration tests appropriately\n\n**Long-term (backlog)**:\n6. Consider shared test fixtures with proper isolation\n\n# Current Status\n\nWe've tagged 16 files with `integration` build tag, but the remaining 279 tests in cmd/bd still take 8+ minutes. This issue tracks fixing the cmd/bd test performance specifically.\n\n# Target\n\nGet `go test ./...` (without `-short` or `-tags=integration`) down to **under 30 seconds**.\n\n\n# THE REAL ROOT CAUSE (Updated Analysis)\n\nAfter examining the actual test code, the problem is clear:\n\n## Every Test Creates Its Own Database From Scratch\n\nLook at `create_test.go`:\n```go\nfunc TestCreate_BasicIssue(t *testing.T) {\n tmpDir := t.TempDir() // ← Creates temp dir\n testDB := filepath.Join(tmpDir, \".beads\", \"beads.db\")\n s := newTestStore(t, testDB) // ← Opens NEW SQLite connection\n // ← Runs migrations\n // ← Sets config\n // ... actual test (3 lines)\n}\n\nfunc TestCreate_WithDescription(t *testing.T) {\n tmpDir := t.TempDir() // ← Creates ANOTHER temp dir\n testDB := filepath.Join(tmpDir, \".beads\", \"beads.db\")\n s := newTestStore(t, testDB) // ← Opens ANOTHER SQLite connection\n // ... actual test (3 lines)\n}\n```\n\n**This happens 279 times!**\n\n## These Tests Don't Need Isolation!\n\nMost tests are just checking:\n- \"Can I create an issue with a title?\"\n- \"Can I create an issue with a description?\"\n- \"Can I add labels?\"\n\nThey don't conflict with each other. They could all share ONE database!\n\n## The Fix: Test Suites with Shared Setup\n\nInstead of:\n```go\nfunc TestCreate_BasicIssue(t *testing.T) {\n s := newTestStore(t, t.TempDir()+\"/db\") // ← Expensive!\n // test\n}\n\nfunc TestCreate_WithDesc(t *testing.T) {\n s := newTestStore(t, t.TempDir()+\"/db\") // ← Expensive!\n // test\n}\n```\n\nDo this:\n```go\nfunc TestCreate(t *testing.T) {\n // ONE setup for all subtests\n s := newTestStore(t, t.TempDir()+\"/db\")\n \n t.Run(\"basic_issue\", func(t *testing.T) {\n t.Parallel() // Can run concurrently - tests don't conflict\n // test using shared `s`\n })\n \n t.Run(\"with_description\", func(t *testing.T) {\n t.Parallel()\n // test using shared `s`\n })\n \n // ... 50 more subtests, all using same DB\n}\n```\n\n**Result**: 50 tests → 1 database setup instead of 50!\n\n## Why This Works\n\nSQLite is fine with concurrent reads and isolated transactions. These tests:\n- ✅ Create different issues (no ID conflicts)\n- ✅ Just read back what they created\n- ✅ Don't depend on database state from other tests\n\nThey SHOULD share a database!\n\n## Real Numbers\n\nCurrent:\n- 279 tests × (create dir + init SQLite + migrations) = **8 minutes**\n\nAfter fix:\n- 10 test suites × (create dir + init SQLite + migrations) = **30 seconds**\n- 279 subtests running in parallel using those 10 DBs = **5 seconds**\n\n**Total: ~35 seconds instead of 8 minutes!**\n\n## Implementation Plan\n\n1. **Group related tests** into suites (Create, List, Update, Delete, etc.)\n2. **One setup per suite** instead of per test\n3. **Use t.Run() for subtests** with t.Parallel()\n4. **Keep tests that actually need isolation** separate (export/import tests, git operations)\n\nThis is way better than shuffling tests into folders!","notes":"## Progress Update (2025-11-21)\n\n✅ **Completed**:\n- Audited all 280 tests, created TEST_SUITE_AUDIT.md ([deleted:bd-c49])\n- Refactored create_test.go to shared DB pattern ([deleted:bd-y6d])\n- Proven the pattern works: 11 tests now run in 0.04s with 1 DB instead of 11\n\n❌ **Current Reality**:\n- Overall test suite: Still 8+ minutes (no meaningful change)\n- Only 1 of 76 test files refactored\n- Saved ~10 DB initializations out of 280\n\n## Acceptance Criteria (REALISTIC)\n\nThis task is NOT complete until:\n- [ ] All P1 files refactored (create ✅, dep, stale, comments, list, ready)\n- [ ] Test suite runs in \u003c 2 minutes\n- [ ] Measured and verified actual speedup\n\n## Next Steps\n\n1. Refactor remaining 5 P1 files: dep_test.go, stale_test.go, comments_test.go, list_test.go, ready_test.go\n2. Measure actual time improvement after each file\n3. Continue with P2 files if needed to hit \u003c2min target","status":"closed","priority":1,"issue_type":"bug","created_at":"2025-11-21T11:37:47.886207-05:00","updated_at":"2025-11-24T00:01:27.554109-08:00","closed_at":"2025-11-23T23:38:54.185364-08:00"} {"id":"bd-1w6i","title":"Document blocked_issues_cache architecture and behavior","description":"Add comprehensive documentation about the blocked_issues_cache optimization to help future maintainers understand the design.\n\n## What to document\n\n1. **Architecture overview**\n - Why cache exists (performance: 752ms -\u003e 29ms)\n - When cache is used (GetReadyWork queries)\n - How cache is maintained (invalidation triggers)\n\n2. **Cache invalidation rules**\n - Invalidated on: dependency add/remove (blocks/parent-child only)\n - Invalidated on: any status change\n - Invalidated on: issue close\n - NOT invalidated on: related/discovered-from dependencies\n\n3. **Transaction safety**\n - All invalidations happen within transactions\n - Cache can use tx or direct db connection\n - Rebuild is atomic (DELETE + INSERT in same tx)\n\n4. **Performance characteristics**\n - Full rebuild on every invalidation\n - Rebuild is fast (\u003c50ms on 10K database)\n - Write complexity traded for read speed\n - Dependency changes are rare vs reads\n\n5. **Edge cases**\n - Parent-child transitive blocking\n - Closed issues don't block\n - Foreign key CASCADE handles deletions\n\n## Where to document\n\nAdd to these locations:\n\n1. **blocked_cache.go** - Add detailed package comment:\n```go\n// Package blocked_cache implements the blocked_issues_cache optimization.\n// \n// Performance: GetReadyWork originally used recursive CTE (752ms on 10K db).\n// With cache: Simple NOT EXISTS query (29ms on 10K db).\n//\n// Cache Maintenance:\n// - Rebuild triggered on dependency changes (blocks/parent-child only)\n// - Rebuild triggered on status changes \n// - Full rebuild (DELETE + INSERT) is fast enough for all cases\n// ...\n```\n\n2. **ARCHITECTURE.md** or **PERFORMANCE.md** (new file) - High-level overview\n\n3. **ready.go** - Update comment at line 84-85 with more detail\n\n## Related\n\n- [deleted:[deleted:[deleted:[deleted:[deleted:[deleted:bd-5qim]]]]]]: GetReadyWork optimization (implemented)\n- bd-13gm: Cache validation tests\n- [deleted:[deleted:[deleted:[deleted:bd-obdc]]]]: Cache rebuild command","status":"closed","priority":3,"issue_type":"task","created_at":"2025-11-23T20:07:23.467296-08:00","updated_at":"2025-11-25T09:10:06.179923-08:00","closed_at":"2025-11-24T01:40:45.872263-08:00"} -{"id":"bd-2bl","title":"Add DeletionRecord type and deletions.jsonl file handling","description":"Parent: bd-imj\n\n## Task\nCreate the core data structures and file I/O for the deletions manifest.\n\n## Implementation\n\n### DeletionRecord struct\n```go\ntype DeletionRecord struct {\n ID string `json:\"id\"`\n Timestamp time.Time `json:\"ts\"`\n Actor string `json:\"by\"`\n Reason string `json:\"reason,omitempty\"`\n}\n```\n\n### File operations\n- `LoadDeletions(path string) (map[string]DeletionRecord, error)` - load into map for O(1) lookup\n- `AppendDeletion(path string, record DeletionRecord) error` - append single record\n- `WriteDeletions(path string, records []DeletionRecord) error` - atomic rewrite for compaction\n\n### Corrupt file recovery\n`LoadDeletions` should:\n- Skip invalid JSON lines with a warning log\n- Return successfully loaded entries\n- Not fail on partial corruption\n- Optionally return count of skipped lines for diagnostics\n\n```go\nfunc LoadDeletions(path string) (map[string]DeletionRecord, int, error) {\n // returns: records, skippedCount, error\n}\n```\n\n### Location\n- New file: `internal/deletions/deletions.go`\n- Path: `.beads/deletions.jsonl`\n\n## Acceptance Criteria\n- [ ] DeletionRecord type with JSON tags\n- [ ] Load/append/write functions with proper error handling\n- [ ] Atomic write (temp file + rename) for compaction safety\n- [ ] Corrupt lines skipped with warning (not fatal)\n- [ ] Unit tests for round-trip serialization\n- [ ] Unit tests for corrupt line handling","status":"open","priority":0,"issue_type":"task","created_at":"2025-11-25T09:56:24.634947-08:00","updated_at":"2025-11-25T10:51:36.405696-08:00"} +{"id":"bd-2bl","title":"Add DeletionRecord type and deletions.jsonl file handling","description":"Parent: bd-imj\n\n## Task\nCreate the core data structures and file I/O for the deletions manifest.\n\n## Implementation\n\n### DeletionRecord struct\n```go\ntype DeletionRecord struct {\n ID string `json:\"id\"`\n Timestamp time.Time `json:\"ts\"`\n Actor string `json:\"by\"`\n Reason string `json:\"reason,omitempty\"`\n}\n```\n\n### File operations\n- `LoadDeletions(path string) (map[string]DeletionRecord, error)` - load into map for O(1) lookup\n- `AppendDeletion(path string, record DeletionRecord) error` - append single record\n- `WriteDeletions(path string, records []DeletionRecord) error` - atomic rewrite for compaction\n\n### Corrupt file recovery\n`LoadDeletions` should:\n- Skip invalid JSON lines with a warning log\n- Return successfully loaded entries\n- Not fail on partial corruption\n- Optionally return count of skipped lines for diagnostics\n\n```go\nfunc LoadDeletions(path string) (map[string]DeletionRecord, int, error) {\n // returns: records, skippedCount, error\n}\n```\n\n### Location\n- New file: `internal/deletions/deletions.go`\n- Path: `.beads/deletions.jsonl`\n\n## Acceptance Criteria\n- [ ] DeletionRecord type with JSON tags\n- [ ] Load/append/write functions with proper error handling\n- [ ] Atomic write (temp file + rename) for compaction safety\n- [ ] Corrupt lines skipped with warning (not fatal)\n- [ ] Unit tests for round-trip serialization\n- [ ] Unit tests for corrupt line handling","status":"closed","priority":0,"issue_type":"task","created_at":"2025-11-25T09:56:24.634947-08:00","updated_at":"2025-11-25T10:59:00.126123-08:00","closed_at":"2025-11-25T10:59:00.126123-08:00"} {"id":"bd-379","title":"Implement `bd setup cursor` for Cursor IDE integration","description":"Create a `bd setup cursor` command that integrates Beads workflow into Cursor IDE via .cursorrules file. Unlike Claude Code (which has hooks), Cursor uses a static rules file to provide context to its AI.","status":"closed","priority":3,"issue_type":"feature","created_at":"2025-11-11T23:32:22.170083-08:00","updated_at":"2025-11-25T09:10:06.181261-08:00","closed_at":"2025-11-24T01:39:26.852836-08:00"} {"id":"bd-3852","title":"Add orphan detection migration","description":"Create migration to detect orphaned children in existing databases. Query: SELECT id FROM issues WHERE id LIKE '%.%' AND substr(id, 1, instr(id || '.', '.') - 1) NOT IN (SELECT id FROM issues). Log results, let user decide action (delete orphans or convert to top-level).","status":"closed","priority":2,"issue_type":"task","created_at":"2025-11-04T12:32:30.727044-08:00","updated_at":"2025-11-24T01:08:18.295118-08:00","closed_at":"2025-11-24T00:31:16.479343-08:00"} {"id":"bd-39o","title":"Rename last_import_hash metadata key to jsonl_content_hash","description":"The metadata key 'last_import_hash' is misleading because it's updated on both import AND export (sync.go:614, import.go:320).\n\nBetter names:\n- jsonl_content_hash (more accurate)\n- last_sync_hash (clearer intent)\n\nThis is a breaking change requiring migration of existing metadata values.","status":"open","priority":2,"issue_type":"task","created_at":"2025-11-20T21:31:07.568739-05:00","updated_at":"2025-11-20T21:31:07.568739-05:00"} diff --git a/internal/deletions/deletions.go b/internal/deletions/deletions.go new file mode 100644 index 00000000..7eea4ea9 --- /dev/null +++ b/internal/deletions/deletions.go @@ -0,0 +1,157 @@ +// Package deletions handles the deletions manifest for tracking deleted issues. +// The deletions.jsonl file is an append-only log that records when issues are +// deleted, enabling propagation of deletions across repo clones via git sync. +package deletions + +import ( + "bufio" + "encoding/json" + "fmt" + "os" + "path/filepath" + "time" +) + +// DeletionRecord represents a single deletion entry in the manifest. +type DeletionRecord struct { + ID string `json:"id"` // Issue ID that was deleted + Timestamp time.Time `json:"ts"` // When the deletion occurred + Actor string `json:"by"` // Who performed the deletion + Reason string `json:"reason,omitempty"` // Optional reason for deletion +} + +// LoadDeletions reads the deletions manifest and returns a map for O(1) lookup. +// It returns the records, the count of skipped (corrupt) lines, and any error. +// Corrupt JSON lines are skipped with a warning rather than failing the load. +func LoadDeletions(path string) (map[string]DeletionRecord, int, error) { + f, err := os.Open(path) // #nosec G304 - controlled path from caller + if err != nil { + if os.IsNotExist(err) { + // No deletions file yet - return empty map + return make(map[string]DeletionRecord), 0, nil + } + return nil, 0, fmt.Errorf("failed to open deletions file: %w", err) + } + defer f.Close() + + records := make(map[string]DeletionRecord) + skipped := 0 + lineNo := 0 + + scanner := bufio.NewScanner(f) + // Allow large lines (up to 1MB) in case of very long reasons + scanner.Buffer(make([]byte, 0, 1024), 1024*1024) + + for scanner.Scan() { + lineNo++ + line := scanner.Text() + if line == "" { + continue + } + + var record DeletionRecord + if err := json.Unmarshal([]byte(line), &record); err != nil { + // Skip corrupt line with warning to stderr + fmt.Fprintf(os.Stderr, "Warning: skipping corrupt line %d in deletions manifest: %v\n", lineNo, err) + skipped++ + continue + } + + // Validate required fields + if record.ID == "" { + fmt.Fprintf(os.Stderr, "Warning: skipping line %d in deletions manifest: missing ID\n", lineNo) + skipped++ + continue + } + + // Use the most recent record for each ID (last write wins) + records[record.ID] = record + } + + if err := scanner.Err(); err != nil { + return nil, skipped, fmt.Errorf("error reading deletions file: %w", err) + } + + return records, skipped, nil +} + +// AppendDeletion appends a single deletion record to the manifest. +// Creates the file if it doesn't exist. +func AppendDeletion(path string, record DeletionRecord) error { + // Ensure directory exists + dir := filepath.Dir(path) + if err := os.MkdirAll(dir, 0755); err != nil { + return fmt.Errorf("failed to create directory: %w", err) + } + + // Open file for appending (create if not exists) + f, err := os.OpenFile(path, os.O_APPEND|os.O_CREATE|os.O_WRONLY, 0644) // #nosec G304 - controlled path + if err != nil { + return fmt.Errorf("failed to open deletions file for append: %w", err) + } + defer f.Close() + + // Marshal record to JSON + data, err := json.Marshal(record) + if err != nil { + return fmt.Errorf("failed to marshal deletion record: %w", err) + } + + // Write line with newline + if _, err := f.Write(append(data, '\n')); err != nil { + return fmt.Errorf("failed to write deletion record: %w", err) + } + + return nil +} + +// WriteDeletions atomically writes the entire deletions manifest. +// Used for compaction to deduplicate and prune old entries. +func WriteDeletions(path string, records []DeletionRecord) error { + // Ensure directory exists + dir := filepath.Dir(path) + if err := os.MkdirAll(dir, 0755); err != nil { + return fmt.Errorf("failed to create directory: %w", err) + } + + // Create temp file in same directory for atomic rename + base := filepath.Base(path) + tempFile, err := os.CreateTemp(dir, base+".tmp.*") + if err != nil { + return fmt.Errorf("failed to create temp file: %w", err) + } + tempPath := tempFile.Name() + defer func() { + _ = tempFile.Close() + _ = os.Remove(tempPath) // Clean up temp file on error + }() + + // Write each record as a JSON line + for _, record := range records { + data, err := json.Marshal(record) + if err != nil { + return fmt.Errorf("failed to marshal deletion record: %w", err) + } + if _, err := tempFile.Write(append(data, '\n')); err != nil { + return fmt.Errorf("failed to write deletion record: %w", err) + } + } + + // Close before rename + if err := tempFile.Close(); err != nil { + return fmt.Errorf("failed to close temp file: %w", err) + } + + // Atomic replace + if err := os.Rename(tempPath, path); err != nil { + return fmt.Errorf("failed to replace deletions file: %w", err) + } + + return nil +} + +// DefaultPath returns the default path for the deletions manifest. +// beadsDir is typically .beads/ +func DefaultPath(beadsDir string) string { + return filepath.Join(beadsDir, "deletions.jsonl") +} diff --git a/internal/deletions/deletions_test.go b/internal/deletions/deletions_test.go new file mode 100644 index 00000000..56067046 --- /dev/null +++ b/internal/deletions/deletions_test.go @@ -0,0 +1,310 @@ +package deletions + +import ( + "os" + "path/filepath" + "testing" + "time" +) + +func TestLoadDeletions_Empty(t *testing.T) { + // Non-existent file should return empty map + records, skipped, err := LoadDeletions("/nonexistent/path/deletions.jsonl") + if err != nil { + t.Fatalf("expected no error for non-existent file, got: %v", err) + } + if skipped != 0 { + t.Errorf("expected 0 skipped, got %d", skipped) + } + if len(records) != 0 { + t.Errorf("expected empty map, got %d records", len(records)) + } +} + +func TestRoundTrip(t *testing.T) { + tmpDir := t.TempDir() + path := filepath.Join(tmpDir, "deletions.jsonl") + + // Create test records + now := time.Now().Truncate(time.Millisecond) // Truncate for JSON round-trip + record1 := DeletionRecord{ + ID: "bd-123", + Timestamp: now, + Actor: "testuser", + Reason: "duplicate", + } + record2 := DeletionRecord{ + ID: "bd-456", + Timestamp: now.Add(time.Hour), + Actor: "testuser", + } + + // Append records + if err := AppendDeletion(path, record1); err != nil { + t.Fatalf("AppendDeletion failed: %v", err) + } + if err := AppendDeletion(path, record2); err != nil { + t.Fatalf("AppendDeletion failed: %v", err) + } + + // Load and verify + records, skipped, err := LoadDeletions(path) + if err != nil { + t.Fatalf("LoadDeletions failed: %v", err) + } + if skipped != 0 { + t.Errorf("expected 0 skipped, got %d", skipped) + } + if len(records) != 2 { + t.Fatalf("expected 2 records, got %d", len(records)) + } + + // Verify record1 + r1, ok := records["bd-123"] + if !ok { + t.Fatal("record bd-123 not found") + } + if r1.Actor != "testuser" { + t.Errorf("expected actor 'testuser', got '%s'", r1.Actor) + } + if r1.Reason != "duplicate" { + t.Errorf("expected reason 'duplicate', got '%s'", r1.Reason) + } + + // Verify record2 + r2, ok := records["bd-456"] + if !ok { + t.Fatal("record bd-456 not found") + } + if r2.Reason != "" { + t.Errorf("expected empty reason, got '%s'", r2.Reason) + } +} + +func TestLoadDeletions_CorruptLines(t *testing.T) { + tmpDir := t.TempDir() + path := filepath.Join(tmpDir, "deletions.jsonl") + + // Write mixed valid and corrupt content + content := `{"id":"bd-001","ts":"2024-01-01T00:00:00Z","by":"user1"} +this is not valid json +{"id":"bd-002","ts":"2024-01-02T00:00:00Z","by":"user2"} +{"broken json +{"id":"bd-003","ts":"2024-01-03T00:00:00Z","by":"user3","reason":"test"} +` + if err := os.WriteFile(path, []byte(content), 0644); err != nil { + t.Fatalf("failed to write test file: %v", err) + } + + records, skipped, err := LoadDeletions(path) + if err != nil { + t.Fatalf("LoadDeletions should not fail on corrupt lines: %v", err) + } + if skipped != 2 { + t.Errorf("expected 2 skipped lines, got %d", skipped) + } + if len(records) != 3 { + t.Errorf("expected 3 valid records, got %d", len(records)) + } + + // Verify valid records were loaded + for _, id := range []string{"bd-001", "bd-002", "bd-003"} { + if _, ok := records[id]; !ok { + t.Errorf("expected record %s to be loaded", id) + } + } +} + +func TestLoadDeletions_MissingID(t *testing.T) { + tmpDir := t.TempDir() + path := filepath.Join(tmpDir, "deletions.jsonl") + + // Write record without ID + content := `{"id":"bd-001","ts":"2024-01-01T00:00:00Z","by":"user1"} +{"ts":"2024-01-02T00:00:00Z","by":"user2"} +{"id":"","ts":"2024-01-03T00:00:00Z","by":"user3"} +` + if err := os.WriteFile(path, []byte(content), 0644); err != nil { + t.Fatalf("failed to write test file: %v", err) + } + + records, skipped, err := LoadDeletions(path) + if err != nil { + t.Fatalf("LoadDeletions failed: %v", err) + } + // Two lines should be skipped: one missing "id" field, one with empty "id" + if skipped != 2 { + t.Errorf("expected 2 skipped lines (missing/empty ID), got %d", skipped) + } + if len(records) != 1 { + t.Errorf("expected 1 valid record, got %d", len(records)) + } +} + +func TestLoadDeletions_LastWriteWins(t *testing.T) { + tmpDir := t.TempDir() + path := filepath.Join(tmpDir, "deletions.jsonl") + + // Write same ID twice with different data + content := `{"id":"bd-001","ts":"2024-01-01T00:00:00Z","by":"user1","reason":"first"} +{"id":"bd-001","ts":"2024-01-02T00:00:00Z","by":"user2","reason":"second"} +` + if err := os.WriteFile(path, []byte(content), 0644); err != nil { + t.Fatalf("failed to write test file: %v", err) + } + + records, skipped, err := LoadDeletions(path) + if err != nil { + t.Fatalf("LoadDeletions failed: %v", err) + } + if skipped != 0 { + t.Errorf("expected 0 skipped, got %d", skipped) + } + if len(records) != 1 { + t.Errorf("expected 1 record (deduplicated), got %d", len(records)) + } + + r := records["bd-001"] + if r.Actor != "user2" { + t.Errorf("expected last write to win (user2), got '%s'", r.Actor) + } + if r.Reason != "second" { + t.Errorf("expected last reason 'second', got '%s'", r.Reason) + } +} + +func TestWriteDeletions_Atomic(t *testing.T) { + tmpDir := t.TempDir() + path := filepath.Join(tmpDir, "deletions.jsonl") + + now := time.Now().Truncate(time.Millisecond) + records := []DeletionRecord{ + {ID: "bd-001", Timestamp: now, Actor: "user1"}, + {ID: "bd-002", Timestamp: now, Actor: "user2", Reason: "cleanup"}, + } + + if err := WriteDeletions(path, records); err != nil { + t.Fatalf("WriteDeletions failed: %v", err) + } + + // Verify by loading + loaded, skipped, err := LoadDeletions(path) + if err != nil { + t.Fatalf("LoadDeletions failed: %v", err) + } + if skipped != 0 { + t.Errorf("expected 0 skipped, got %d", skipped) + } + if len(loaded) != 2 { + t.Errorf("expected 2 records, got %d", len(loaded)) + } +} + +func TestWriteDeletions_Overwrite(t *testing.T) { + tmpDir := t.TempDir() + path := filepath.Join(tmpDir, "deletions.jsonl") + + now := time.Now().Truncate(time.Millisecond) + + // Write initial records + initial := []DeletionRecord{ + {ID: "bd-001", Timestamp: now, Actor: "user1"}, + {ID: "bd-002", Timestamp: now, Actor: "user2"}, + {ID: "bd-003", Timestamp: now, Actor: "user3"}, + } + if err := WriteDeletions(path, initial); err != nil { + t.Fatalf("initial WriteDeletions failed: %v", err) + } + + // Overwrite with fewer records (simulates compaction pruning) + compacted := []DeletionRecord{ + {ID: "bd-002", Timestamp: now, Actor: "user2"}, + } + if err := WriteDeletions(path, compacted); err != nil { + t.Fatalf("compacted WriteDeletions failed: %v", err) + } + + // Verify only compacted records remain + loaded, _, err := LoadDeletions(path) + if err != nil { + t.Fatalf("LoadDeletions failed: %v", err) + } + if len(loaded) != 1 { + t.Errorf("expected 1 record after compaction, got %d", len(loaded)) + } + if _, ok := loaded["bd-002"]; !ok { + t.Error("expected bd-002 to remain after compaction") + } +} + +func TestAppendDeletion_CreatesDirectory(t *testing.T) { + tmpDir := t.TempDir() + path := filepath.Join(tmpDir, "nested", "dir", "deletions.jsonl") + + record := DeletionRecord{ + ID: "bd-001", + Timestamp: time.Now(), + Actor: "testuser", + } + + if err := AppendDeletion(path, record); err != nil { + t.Fatalf("AppendDeletion should create parent directories: %v", err) + } + + // Verify file exists + if _, err := os.Stat(path); err != nil { + t.Errorf("file should exist after append: %v", err) + } +} + +func TestWriteDeletions_CreatesDirectory(t *testing.T) { + tmpDir := t.TempDir() + path := filepath.Join(tmpDir, "nested", "dir", "deletions.jsonl") + + records := []DeletionRecord{ + {ID: "bd-001", Timestamp: time.Now(), Actor: "testuser"}, + } + + if err := WriteDeletions(path, records); err != nil { + t.Fatalf("WriteDeletions should create parent directories: %v", err) + } + + // Verify file exists + if _, err := os.Stat(path); err != nil { + t.Errorf("file should exist after write: %v", err) + } +} + +func TestDefaultPath(t *testing.T) { + path := DefaultPath("/home/user/project/.beads") + expected := "/home/user/project/.beads/deletions.jsonl" + if path != expected { + t.Errorf("expected %s, got %s", expected, path) + } +} + +func TestLoadDeletions_EmptyLines(t *testing.T) { + tmpDir := t.TempDir() + path := filepath.Join(tmpDir, "deletions.jsonl") + + // Write content with empty lines + content := `{"id":"bd-001","ts":"2024-01-01T00:00:00Z","by":"user1"} + +{"id":"bd-002","ts":"2024-01-02T00:00:00Z","by":"user2"} + +` + if err := os.WriteFile(path, []byte(content), 0644); err != nil { + t.Fatalf("failed to write test file: %v", err) + } + + records, skipped, err := LoadDeletions(path) + if err != nil { + t.Fatalf("LoadDeletions failed: %v", err) + } + if skipped != 0 { + t.Errorf("empty lines should not count as skipped, got %d", skipped) + } + if len(records) != 2 { + t.Errorf("expected 2 records, got %d", len(records)) + } +}