From be784a0b4b3398e8dd915cfe49342a05ff4c7928 Mon Sep 17 00:00:00 2001 From: Steve Yegge Date: Tue, 25 Nov 2025 11:46:06 -0800 Subject: [PATCH] bd sync: 2025-11-25 11:46:06 --- .beads/beads.jsonl | 4 +- cmd/bd/cleanup.go | 2 +- cmd/bd/delete.go | 89 +++++++++++- cmd/bd/delete_recording_test.go | 223 ++++++++++++++++++++++++++++++ cmd/bd/import_shared.go | 4 + internal/importer/importer.go | 80 +++++++++++ internal/importer/purge_test.go | 233 ++++++++++++++++++++++++++++++++ 7 files changed, 630 insertions(+), 5 deletions(-) create mode 100644 cmd/bd/delete_recording_test.go create mode 100644 internal/importer/purge_test.go diff --git a/.beads/beads.jsonl b/.beads/beads.jsonl index 0556ed42..969f528c 100644 --- a/.beads/beads.jsonl +++ b/.beads/beads.jsonl @@ -46,8 +46,8 @@ {"id":"bd-gfu","title":"Add --start flag to bd daemon, show help with no args","description":"Currently `bd daemon` with no args immediately starts the daemon. This is inconsistent with other daemon management commands like `--stop`, `--status`, etc.\n\n## Proposed Changes\n\n1. Add `--start` flag to explicitly start daemon\n2. With no args or flags, print help text showing available options\n3. Maintain backward compatibility where feasible\n\n## Current Consumers\n\n- **Auto-start logic** (`daemon_autostart.go:106, 270`): Calls `bd daemon` programmatically - needs update\n- **MCP docs** (SETUP_DAEMON.md:111): Already incorrectly shows `bd daemon start` - will be fixed by this change\n- **Python daemon client**: Suggests `bd daemon` in error messages - needs doc update\n\n## Implementation Notes\n\n- Default behavior (no args/flags) should show help\n- `--start` should start daemon (current no-args behavior)\n- Auto-start code needs to pass `--start` flag explicitly\n- Update all documentation to use `bd daemon --start`\n- Ensure backward compat doesn't break existing workflows","status":"closed","priority":2,"issue_type":"feature","created_at":"2025-11-23T23:54:49.906553-08:00","updated_at":"2025-11-24T00:03:15.231345-08:00","closed_at":"2025-11-24T00:03:15.231345-08:00"} {"id":"bd-gqo","title":"Implement health checks in daemon event loop","description":"Add health checks to checkDaemonHealth() function in daemon_event_loop.go:170:\n- Database integrity check\n- Disk space check\n- Memory usage check\n\nCurrently it's just a no-op placeholder.","status":"open","priority":3,"issue_type":"feature","created_at":"2025-11-21T18:55:07.534304-05:00","updated_at":"2025-11-21T18:55:07.534304-05:00"} {"id":"bd-hdt","title":"Implement auto-merge functionality in duplicates command","description":"The duplicates.go file has a TODO at line 95 to implement the performMerge function for automatic duplicate merging. Currently it just prints a warning message. This would automate the merge process instead of just suggesting commands.","status":"open","priority":2,"issue_type":"feature","created_at":"2025-11-21T18:55:02.828619-05:00","updated_at":"2025-11-21T18:55:02.828619-05:00"} -{"id":"bd-ibg","title":"Integrate deletions manifest into import logic","description":"Parent: bd-imj\n\n## Task\nModify import to check deletions manifest and delete stale DB issues.\n\n## Implementation\n\nIn `internal/importer/importer.go` (or equivalent):\n\n```go\nfunc (i *Importer) Import(jsonlPath string) error {\n jsonlIDs := i.parseJSONL(jsonlPath)\n deletions, _ := deletions.Load(\".beads/deletions.jsonl\")\n \n for _, dbID := range i.db.AllIDs() {\n if jsonlIDs.Has(dbID) {\n continue // In sync\n }\n if del, ok := deletions[dbID]; ok {\n i.db.Delete(dbID)\n log.Printf(\"Purged %s (deleted %s by %s)\", dbID, del.Timestamp, del.Actor)\n continue\n }\n // Not deleted, keep as local work\n }\n \n // Normal create/update logic...\n}\n```\n\n## Acceptance Criteria\n- [ ] Import loads deletions manifest\n- [ ] DB issues not in JSONL but in manifest are deleted\n- [ ] Deletion logged with metadata (timestamp, actor)\n- [ ] Handles missing/empty deletions file gracefully\n- [ ] Integration test: delete in clone A propagates to clone B","status":"in_progress","priority":0,"issue_type":"task","created_at":"2025-11-25T09:56:25.168983-08:00","updated_at":"2025-11-25T11:32:42.188441-08:00","dependencies":[{"issue_id":"bd-ibg","depends_on_id":"bd-2bl","type":"blocks","created_at":"2025-11-25T09:57:49.904629-08:00","created_by":"daemon"}]} -{"id":"bd-ieh","title":"Record deletions in bd delete and bd cleanup","description":"Parent: bd-imj\n\n## Task\nModify delete operations to append to deletions manifest.\n\n## Implementation\n\n### Order of Operations (IMPORTANT)\n1. Append to deletions.jsonl first (can fail safely, no side effects)\n2. Delete from DB\n3. Export to JSONL\n\nThis order ensures we never lose deletion records.\n\n### bd delete\n```go\nfunc (s *Storage) DeleteIssue(id, actor, reason string) error {\n // NEW: Record in manifest FIRST\n if err := deletions.Append(\".beads/deletions.jsonl\", DeletionRecord{\n ID: id,\n Timestamp: time.Now().UTC(),\n Actor: actor,\n Reason: reason,\n }); err != nil {\n return fmt.Errorf(\"failed to record deletion: %w\", err)\n }\n \n // Then delete from DB\n s.db.Delete(id)\n \n // Then export\n // ...\n}\n```\n\n### bd cleanup\nSame pattern, with reason=\"cleanup\"\n\n### Actor sourcing (in order)\n1. `--actor` flag if provided\n2. `$BD_ACTOR` env var if set\n3. `git config user.name` (preferred default)\n4. `$USER` as last resort\n\n## Acceptance Criteria\n- [ ] `bd delete` appends to deletions.jsonl BEFORE deleting\n- [ ] `bd cleanup` appends to deletions.jsonl (one entry per deleted issue)\n- [ ] Actor sourced from flag \u003e env \u003e git config \u003e $USER\n- [ ] Reason field populated appropriately\n- [ ] Test: delete creates manifest entry with correct metadata\n- [ ] Test: failed manifest append prevents deletion","status":"in_progress","priority":0,"issue_type":"task","created_at":"2025-11-25T09:56:25.721739-08:00","updated_at":"2025-11-25T11:32:42.128789-08:00","dependencies":[{"issue_id":"bd-ieh","depends_on_id":"bd-2bl","type":"blocks","created_at":"2025-11-25T09:57:49.940751-08:00","created_by":"daemon"}]} +{"id":"bd-ibg","title":"Integrate deletions manifest into import logic","description":"Parent: bd-imj\n\n## Task\nModify import to check deletions manifest and delete stale DB issues.\n\n## Implementation\n\nIn `internal/importer/importer.go` (or equivalent):\n\n```go\nfunc (i *Importer) Import(jsonlPath string) error {\n jsonlIDs := i.parseJSONL(jsonlPath)\n deletions, _ := deletions.Load(\".beads/deletions.jsonl\")\n \n for _, dbID := range i.db.AllIDs() {\n if jsonlIDs.Has(dbID) {\n continue // In sync\n }\n if del, ok := deletions[dbID]; ok {\n i.db.Delete(dbID)\n log.Printf(\"Purged %s (deleted %s by %s)\", dbID, del.Timestamp, del.Actor)\n continue\n }\n // Not deleted, keep as local work\n }\n \n // Normal create/update logic...\n}\n```\n\n## Acceptance Criteria\n- [ ] Import loads deletions manifest\n- [ ] DB issues not in JSONL but in manifest are deleted\n- [ ] Deletion logged with metadata (timestamp, actor)\n- [ ] Handles missing/empty deletions file gracefully\n- [ ] Integration test: delete in clone A propagates to clone B","status":"closed","priority":0,"issue_type":"task","created_at":"2025-11-25T09:56:25.168983-08:00","updated_at":"2025-11-25T11:45:48.824571-08:00","closed_at":"2025-11-25T11:45:48.824571-08:00","dependencies":[{"issue_id":"bd-ibg","depends_on_id":"bd-2bl","type":"blocks","created_at":"2025-11-25T09:57:49.904629-08:00","created_by":"daemon"}]} +{"id":"bd-ieh","title":"Record deletions in bd delete and bd cleanup","description":"Parent: bd-imj\n\n## Task\nModify delete operations to append to deletions manifest.\n\n## Implementation\n\n### Order of Operations (IMPORTANT)\n1. Append to deletions.jsonl first (can fail safely, no side effects)\n2. Delete from DB\n3. Export to JSONL\n\nThis order ensures we never lose deletion records.\n\n### bd delete\n```go\nfunc (s *Storage) DeleteIssue(id, actor, reason string) error {\n // NEW: Record in manifest FIRST\n if err := deletions.Append(\".beads/deletions.jsonl\", DeletionRecord{\n ID: id,\n Timestamp: time.Now().UTC(),\n Actor: actor,\n Reason: reason,\n }); err != nil {\n return fmt.Errorf(\"failed to record deletion: %w\", err)\n }\n \n // Then delete from DB\n s.db.Delete(id)\n \n // Then export\n // ...\n}\n```\n\n### bd cleanup\nSame pattern, with reason=\"cleanup\"\n\n### Actor sourcing (in order)\n1. `--actor` flag if provided\n2. `$BD_ACTOR` env var if set\n3. `git config user.name` (preferred default)\n4. `$USER` as last resort\n\n## Acceptance Criteria\n- [ ] `bd delete` appends to deletions.jsonl BEFORE deleting\n- [ ] `bd cleanup` appends to deletions.jsonl (one entry per deleted issue)\n- [ ] Actor sourced from flag \u003e env \u003e git config \u003e $USER\n- [ ] Reason field populated appropriately\n- [ ] Test: delete creates manifest entry with correct metadata\n- [ ] Test: failed manifest append prevents deletion","status":"closed","priority":0,"issue_type":"task","created_at":"2025-11-25T09:56:25.721739-08:00","updated_at":"2025-11-25T11:45:48.77157-08:00","closed_at":"2025-11-25T11:45:48.77157-08:00","dependencies":[{"issue_id":"bd-ieh","depends_on_id":"bd-2bl","type":"blocks","created_at":"2025-11-25T09:57:49.940751-08:00","created_by":"daemon"}]} {"id":"bd-imj","title":"Deletion propagation via deletions manifest","description":"## Problem\n\nWhen `bd cleanup -f` or `bd delete` removes issues in one clone, those deletions don't propagate to other clones. The import logic only creates/updates, never deletes. This causes \"resurrection\" where deleted issues reappear.\n\n## Root Cause\n\nImport sees DB issues not in JSONL and assumes they're \"local unpushed work\" rather than \"intentionally deleted upstream.\"\n\n## Solution: Deletions Manifest\n\nAdd `.beads/deletions.jsonl` - an append-only log of deleted issue IDs with metadata.\n\n### Format\n```jsonl\n{\"id\":\"bd-xxx\",\"ts\":\"2025-11-25T10:00:00Z\",\"by\":\"stevey\"}\n{\"id\":\"bd-yyy\",\"ts\":\"2025-11-25T10:05:00Z\",\"by\":\"claude\",\"reason\":\"duplicate of bd-zzz\"}\n```\n\n### Fields\n- `id`: Issue ID (required)\n- `ts`: ISO 8601 UTC timestamp (required)\n- `by`: Actor who deleted (required)\n- `reason`: Optional context (\"cleanup\", \"duplicate of X\", etc.)\n\n### Import Logic\n```\nFor each DB issue not in JSONL:\n 1. Check deletions manifest → if found, delete from DB\n 2. Fallback: check git history → if found, delete + backfill manifest\n 3. Neither → keep (local unpushed work)\n```\n\n### Conflict Resolution\nSimultaneous deletions from multiple clones are handled naturally:\n- Append-only design means both clones append their deletion records\n- On merge, file contains duplicate entries (same ID, different timestamps)\n- `LoadDeletions` deduplicates by ID (keeps any/first entry)\n- Result: deletion propagates correctly, duplicates are harmless\n\n### Pruning Policy\n- Default retention: 7 days (configurable via `deletions.retention_days`)\n- Auto-compact during `bd sync` is **opt-in** (disabled by default)\n- Hard cap: `deletions.max_entries` (default 50000)\n- Git fallback handles pruned entries (self-healing)\n\n### Self-Healing\nWhen git fallback catches a resurrection (pruned entry), it backfills the manifest. One-time git scan cost, then fast again.\n\n### Size Estimates\n- ~80 bytes/entry\n- 7-day retention with 100 deletions/day = ~56KB\n- Git compressed: ~10KB\n\n## Benefits\n- ✅ Deletions propagate across clones\n- ✅ O(1) lookup (no git scan in normal case)\n- ✅ Works in shallow clones\n- ✅ Survives history rewrite\n- ✅ Audit trail (who deleted what when)\n- ✅ Self-healing via git fallback\n- ✅ Bounded size via time-based pruning\n\n## References\n- Investigation session: 2025-11-25\n- Related: bd-2q6d (stale database warnings)","status":"open","priority":0,"issue_type":"epic","created_at":"2025-11-25T09:56:01.98027-08:00","updated_at":"2025-11-25T10:52:28.738368-08:00","dependencies":[{"issue_id":"bd-imj","depends_on_id":"bd-2bl","type":"blocks","created_at":"2025-11-25T09:57:37.845868-08:00","created_by":"daemon"},{"issue_id":"bd-imj","depends_on_id":"bd-ibg","type":"blocks","created_at":"2025-11-25T09:57:42.694905-08:00","created_by":"daemon"},{"issue_id":"bd-imj","depends_on_id":"bd-ieh","type":"blocks","created_at":"2025-11-25T09:57:42.724272-08:00","created_by":"daemon"},{"issue_id":"bd-imj","depends_on_id":"bd-pnm","type":"blocks","created_at":"2025-11-25T09:57:42.755707-08:00","created_by":"daemon"},{"issue_id":"bd-imj","depends_on_id":"bd-v2x","type":"blocks","created_at":"2025-11-25T09:57:42.790453-08:00","created_by":"daemon"},{"issue_id":"bd-imj","depends_on_id":"bd-qsm","type":"blocks","created_at":"2025-11-25T09:57:42.821911-08:00","created_by":"daemon"},{"issue_id":"bd-imj","depends_on_id":"bd-x2i","type":"blocks","created_at":"2025-11-25T09:57:42.851712-08:00","created_by":"daemon"},{"issue_id":"bd-imj","depends_on_id":"bd-44e","type":"blocks","created_at":"2025-11-25T09:57:42.88154-08:00","created_by":"daemon"},{"issue_id":"bd-imj","depends_on_id":"bd-kzo","type":"blocks","created_at":"2025-11-25T10:52:16.319402-08:00","created_by":"daemon"}]} {"id":"bd-j3zt","title":"Fix mypy errors in beads-mcp","description":"Running `mypy .` in `integrations/beads-mcp` reports 287 errors. These should be addressed to improve type safety and code quality.","status":"open","priority":3,"issue_type":"task","created_at":"2025-11-20T18:53:28.557708-05:00","updated_at":"2025-11-20T18:53:28.557708-05:00"} {"id":"bd-koab","title":"Import should continue on FOREIGN KEY constraint violations from deletions","description":"# Problem\n\nWhen importing JSONL after a merge that includes deletions, we may encounter FOREIGN KEY constraint violations if:\n- Issue A was deleted in one branch\n- Issue B (that depends on A) was modified in another branch \n- The merge keeps the deletion of A and the modification of B\n- Import tries to import B with a dependency/reference to deleted A\n\nCurrently import fails completely on such constraint violations, requiring manual intervention.\n\n# Solution\n\nAdd IsForeignKeyConstraintError() helper similar to IsUniqueConstraintError()\n\nUpdate import code to:\n1. Detect FOREIGN KEY constraint violations\n2. Log a warning with the issue ID and constraint\n3. Continue importing remaining issues\n4. Report summary of skipped issues at the end\n\n# Implementation Notes\n\n- Add to internal/storage/sqlite/util.go\n- Pattern: strings.Contains(err.Error(), \"FOREIGN KEY constraint failed\")\n- Update importer to handle these errors gracefully\n- Keep track of skipped issues for summary reporting","notes":"## Implementation Complete\n\nAdded FOREIGN KEY constraint violation handling to the importer:\n\n**Changes made:**\n\n1. **internal/importer/importer.go**\n - Added SkippedDependencies field to Result struct\n - Updated importDependencies() to accept result parameter\n - Added FK constraint detection using sqlite.IsForeignKeyConstraintError()\n - Log warning for each skipped dependency\n - Track skipped dependencies in result\n\n2. **cmd/bd/import_shared.go**\n - Added SkippedDependencies field to ImportResult struct\n - Updated result conversion to include skipped dependencies\n\n3. **cmd/bd/import.go**\n - Added summary reporting for skipped dependencies\n - Displays warning with list of skipped dependencies and helpful context\n\n**Behavior:**\n- When a FOREIGN KEY constraint violation is encountered during dependency import:\n - A warning is logged: 'Warning: Skipping dependency due to missing reference: issue-a → issue-b (blocks)'\n - The dependency is tracked in result.SkippedDependencies\n - Import continues with remaining dependencies\n - Summary at end lists all skipped dependencies with context message\n\n**Testing:**\n- All existing importer tests pass\n- Build succeeds\n- Ready for real-world testing when FK constraint violations are encountered","status":"closed","priority":2,"issue_type":"feature","created_at":"2025-11-23T21:37:02.811665-08:00","updated_at":"2025-11-24T00:01:27.559495-08:00","closed_at":"2025-11-23T23:31:04.325337-08:00"} diff --git a/cmd/bd/cleanup.go b/cmd/bd/cleanup.go index 501ef0de..d0586b6c 100644 --- a/cmd/bd/cleanup.go +++ b/cmd/bd/cleanup.go @@ -127,7 +127,7 @@ SEE ALSO: } // Use the existing batch deletion logic - deleteBatch(cmd, issueIDs, force, dryRun, cascade, jsonOutput) + deleteBatch(cmd, issueIDs, force, dryRun, cascade, jsonOutput, "cleanup") }, } diff --git a/cmd/bd/delete.go b/cmd/bd/delete.go index 7f1825ad..d0b50e3e 100644 --- a/cmd/bd/delete.go +++ b/cmd/bd/delete.go @@ -1,14 +1,20 @@ package main + import ( "bufio" "context" "encoding/json" "fmt" "os" + "os/exec" + "path/filepath" "regexp" "strings" + "time" + "github.com/fatih/color" "github.com/spf13/cobra" + "github.com/steveyegge/beads/internal/deletions" "github.com/steveyegge/beads/internal/storage/sqlite" "github.com/steveyegge/beads/internal/types" ) @@ -62,7 +68,7 @@ Force: Delete and orphan dependents issueIDs = uniqueStrings(issueIDs) // Handle batch deletion if len(issueIDs) > 1 { - deleteBatch(cmd, issueIDs, force, dryRun, cascade, jsonOutput) + deleteBatch(cmd, issueIDs, force, dryRun, cascade, jsonOutput, "batch delete") return } // Single issue deletion (legacy behavior) @@ -161,6 +167,13 @@ Force: Delete and orphan dependents return } // Actually delete + // 0. Record deletion in manifest FIRST (before any DB changes) + // This ensures deletion propagates via git sync even if DB operations fail + deleteActor := getActorWithGit() + if err := recordDeletion(issueID, deleteActor, "manual delete"); err != nil { + fmt.Fprintf(os.Stderr, "Error: failed to record deletion: %v\n", err) + os.Exit(1) + } // 1. Update text references in connected issues (all text fields) updatedIssueCount := 0 for id, connIssue := range connectedIssues { @@ -319,7 +332,7 @@ func removeIssueFromJSONL(issueID string) error { } // deleteBatch handles deletion of multiple issues //nolint:unparam // cmd parameter required for potential future use -func deleteBatch(_ *cobra.Command, issueIDs []string, force bool, dryRun bool, cascade bool, jsonOutput bool) { +func deleteBatch(_ *cobra.Command, issueIDs []string, force bool, dryRun bool, cascade bool, jsonOutput bool, reason string) { // Ensure we have a direct store when daemon lacks delete support if daemonClient != nil { if err := ensureDirectMode("daemon does not support delete command"); err != nil { @@ -414,6 +427,13 @@ func deleteBatch(_ *cobra.Command, issueIDs []string, force bool, dryRun bool, c } } } + // Record deletions in manifest FIRST (before any DB changes) + // This ensures deletion propagates via git sync even if DB operations fail + deleteActor := getActorWithGit() + if err := recordDeletions(issueIDs, deleteActor, reason); err != nil { + fmt.Fprintf(os.Stderr, "Error: failed to record deletions: %v\n", err) + os.Exit(1) + } // Actually delete result, err := d.DeleteIssues(ctx, issueIDs, cascade, force, false) if err != nil { @@ -553,6 +573,71 @@ func uniqueStrings(slice []string) []string { } return result } + +// getActorWithGit returns the actor for audit trail with git config fallback. +// Priority: global actor var (from --actor flag or BD_ACTOR env) > git config user.name > $USER > "unknown" +func getActorWithGit() string { + // If actor is already set (from flag or env), use it + if actor != "" && actor != "unknown" { + return actor + } + + // Try git config user.name + cmd := exec.Command("git", "config", "user.name") + if output, err := cmd.Output(); err == nil { + if gitUser := strings.TrimSpace(string(output)); gitUser != "" { + return gitUser + } + } + + // Fall back to USER env + if user := os.Getenv("USER"); user != "" { + return user + } + + return "unknown" +} + +// getDeletionsPath returns the path to the deletions manifest file. +// Uses the same directory as the database. +func getDeletionsPath() string { + // Get the .beads directory from dbPath + beadsDir := filepath.Dir(dbPath) + return deletions.DefaultPath(beadsDir) +} + +// recordDeletion appends a deletion record to the deletions manifest. +// This MUST be called BEFORE deleting from the database to ensure +// deletion records are never lost. +func recordDeletion(id, deleteActor, reason string) error { + record := deletions.DeletionRecord{ + ID: id, + Timestamp: time.Now().UTC(), + Actor: deleteActor, + Reason: reason, + } + return deletions.AppendDeletion(getDeletionsPath(), record) +} + +// recordDeletions appends multiple deletion records to the deletions manifest. +// This MUST be called BEFORE deleting from the database to ensure +// deletion records are never lost. +func recordDeletions(ids []string, deleteActor, reason string) error { + path := getDeletionsPath() + for _, id := range ids { + record := deletions.DeletionRecord{ + ID: id, + Timestamp: time.Now().UTC(), + Actor: deleteActor, + Reason: reason, + } + if err := deletions.AppendDeletion(path, record); err != nil { + return fmt.Errorf("failed to record deletion for %s: %w", id, err) + } + } + return nil +} + func init() { deleteCmd.Flags().BoolP("force", "f", false, "Actually delete (without this flag, shows preview)") deleteCmd.Flags().String("from-file", "", "Read issue IDs from file (one per line)") diff --git a/cmd/bd/delete_recording_test.go b/cmd/bd/delete_recording_test.go new file mode 100644 index 00000000..af85bc2f --- /dev/null +++ b/cmd/bd/delete_recording_test.go @@ -0,0 +1,223 @@ +package main + +import ( + "context" + "os" + "path/filepath" + "testing" + "time" + + "github.com/steveyegge/beads/internal/deletions" + "github.com/steveyegge/beads/internal/storage/sqlite" + "github.com/steveyegge/beads/internal/types" +) + +// TestRecordDeletion tests that recordDeletion creates deletion manifest entries +func TestRecordDeletion(t *testing.T) { + tmpDir := t.TempDir() + + // Set up dbPath so getDeletionsPath() works + oldDbPath := dbPath + dbPath = filepath.Join(tmpDir, "beads.db") + defer func() { dbPath = oldDbPath }() + + // Create the .beads directory + if err := os.MkdirAll(tmpDir, 0750); err != nil { + t.Fatalf("failed to create directory: %v", err) + } + + // Test recordDeletion + err := recordDeletion("test-abc", "test-user", "test reason") + if err != nil { + t.Fatalf("recordDeletion failed: %v", err) + } + + // Verify the deletion was recorded + deletionsPath := getDeletionsPath() + result, err := deletions.LoadDeletions(deletionsPath) + if err != nil { + t.Fatalf("LoadDeletions failed: %v", err) + } + + if len(result.Records) != 1 { + t.Fatalf("expected 1 deletion record, got %d", len(result.Records)) + } + + del, found := result.Records["test-abc"] + if !found { + t.Fatalf("deletion record for 'test-abc' not found") + } + + if del.Actor != "test-user" { + t.Errorf("expected actor 'test-user', got '%s'", del.Actor) + } + + if del.Reason != "test reason" { + t.Errorf("expected reason 'test reason', got '%s'", del.Reason) + } + + // Timestamp should be recent (within last minute) + if time.Since(del.Timestamp) > time.Minute { + t.Errorf("timestamp seems too old: %v", del.Timestamp) + } +} + +// TestRecordDeletions tests that recordDeletions creates multiple deletion manifest entries +func TestRecordDeletions(t *testing.T) { + tmpDir := t.TempDir() + + // Set up dbPath so getDeletionsPath() works + oldDbPath := dbPath + dbPath = filepath.Join(tmpDir, "beads.db") + defer func() { dbPath = oldDbPath }() + + // Create the .beads directory + if err := os.MkdirAll(tmpDir, 0750); err != nil { + t.Fatalf("failed to create directory: %v", err) + } + + // Test recordDeletions with multiple IDs + ids := []string{"test-abc", "test-def", "test-ghi"} + err := recordDeletions(ids, "batch-user", "batch cleanup") + if err != nil { + t.Fatalf("recordDeletions failed: %v", err) + } + + // Verify the deletions were recorded + deletionsPath := getDeletionsPath() + result, err := deletions.LoadDeletions(deletionsPath) + if err != nil { + t.Fatalf("LoadDeletions failed: %v", err) + } + + if len(result.Records) != 3 { + t.Fatalf("expected 3 deletion records, got %d", len(result.Records)) + } + + for _, id := range ids { + del, found := result.Records[id] + if !found { + t.Errorf("deletion record for '%s' not found", id) + continue + } + + if del.Actor != "batch-user" { + t.Errorf("expected actor 'batch-user' for %s, got '%s'", id, del.Actor) + } + + if del.Reason != "batch cleanup" { + t.Errorf("expected reason 'batch cleanup' for %s, got '%s'", id, del.Reason) + } + } +} + +// TestGetActorWithGit tests actor sourcing logic +func TestGetActorWithGit(t *testing.T) { + // Save original actor value + oldActor := actor + defer func() { actor = oldActor }() + + // Test case 1: actor is set from flag/env + actor = "flag-user" + result := getActorWithGit() + if result != "flag-user" { + t.Errorf("expected 'flag-user' when actor is set, got '%s'", result) + } + + // Test case 2: actor is "unknown" - should try git config + actor = "unknown" + result = getActorWithGit() + // Can't test exact result since it depends on git config, but it shouldn't be empty + if result == "" { + t.Errorf("expected non-empty result when actor is 'unknown'") + } + + // Test case 3: actor is empty - should try git config + actor = "" + result = getActorWithGit() + if result == "" { + t.Errorf("expected non-empty result when actor is empty") + } +} + +// TestDeleteRecordingOrderOfOperations verifies deletion is recorded before DB delete +func TestDeleteRecordingOrderOfOperations(t *testing.T) { + ctx := context.Background() + tmpDir := t.TempDir() + + // Set up dbPath + oldDbPath := dbPath + dbPath = filepath.Join(tmpDir, "beads.db") + defer func() { dbPath = oldDbPath }() + + // Create database + testStore, err := sqlite.New(ctx, dbPath) + if err != nil { + t.Fatalf("failed to create database: %v", err) + } + defer testStore.Close() + + // Initialize prefix + if err := testStore.SetConfig(ctx, "issue_prefix", "test"); err != nil { + t.Fatalf("failed to set prefix: %v", err) + } + + // Create an issue + issue := &types.Issue{ + ID: "test-delete-order", + Title: "Test Order of Operations", + Status: types.StatusOpen, + Priority: 1, + IssueType: types.TypeTask, + } + if err := testStore.CreateIssue(ctx, issue, "test"); err != nil { + t.Fatalf("failed to create issue: %v", err) + } + + // Record deletion (simulating what delete command does) + if err := recordDeletion(issue.ID, "test-user", "order test"); err != nil { + t.Fatalf("recordDeletion failed: %v", err) + } + + // Verify record was created BEFORE any DB changes + deletionsPath := getDeletionsPath() + result, err := deletions.LoadDeletions(deletionsPath) + if err != nil { + t.Fatalf("LoadDeletions failed: %v", err) + } + + if _, found := result.Records[issue.ID]; !found { + t.Error("deletion record should exist before DB deletion") + } + + // Now verify the issue still exists in DB (we only recorded, didn't delete) + existing, err := testStore.GetIssue(ctx, issue.ID) + if err != nil { + t.Fatalf("GetIssue failed: %v", err) + } + if existing == nil { + t.Error("issue should still exist in DB (we only recorded the deletion)") + } + + // Now delete from DB + if err := testStore.DeleteIssue(ctx, issue.ID); err != nil { + t.Fatalf("DeleteIssue failed: %v", err) + } + + // Verify both: deletion record exists AND issue is gone from DB + result, err = deletions.LoadDeletions(deletionsPath) + if err != nil { + t.Fatalf("LoadDeletions failed: %v", err) + } + if _, found := result.Records[issue.ID]; !found { + t.Error("deletion record should still exist after DB deletion") + } + + existing, err = testStore.GetIssue(ctx, issue.ID) + if err != nil { + t.Fatalf("GetIssue failed: %v", err) + } + if existing != nil { + t.Error("issue should be gone from DB after deletion") + } +} diff --git a/cmd/bd/import_shared.go b/cmd/bd/import_shared.go index 863d8fd3..1b1665a9 100644 --- a/cmd/bd/import_shared.go +++ b/cmd/bd/import_shared.go @@ -180,6 +180,8 @@ type ImportResult struct { ExpectedPrefix string // Database configured prefix MismatchPrefixes map[string]int // Map of mismatched prefixes to count SkippedDependencies []string // Dependencies skipped due to FK constraint violations + Purged int // Issues purged from DB (found in deletions manifest) + PurgedIDs []string // IDs that were purged } // importIssuesCore handles the core import logic used by both manual and auto-import. @@ -240,6 +242,8 @@ func importIssuesCore(ctx context.Context, dbPath string, store storage.Storage, ExpectedPrefix: result.ExpectedPrefix, MismatchPrefixes: result.MismatchPrefixes, SkippedDependencies: result.SkippedDependencies, + Purged: result.Purged, + PurgedIDs: result.PurgedIDs, }, nil } diff --git a/internal/importer/importer.go b/internal/importer/importer.go index 978b33d7..7b854e86 100644 --- a/internal/importer/importer.go +++ b/internal/importer/importer.go @@ -4,9 +4,11 @@ import ( "context" "fmt" "os" + "path/filepath" "sort" "strings" + "github.com/steveyegge/beads/internal/deletions" "github.com/steveyegge/beads/internal/storage" "github.com/steveyegge/beads/internal/storage/sqlite" "github.com/steveyegge/beads/internal/types" @@ -51,6 +53,8 @@ type Result struct { ExpectedPrefix string // Database configured prefix MismatchPrefixes map[string]int // Map of mismatched prefixes to count SkippedDependencies []string // Dependencies skipped due to FK constraint violations + Purged int // Issues purged from DB (found in deletions manifest) + PurgedIDs []string // IDs that were purged } // ImportIssues handles the core import logic used by both manual and auto-import. @@ -144,6 +148,15 @@ func ImportIssues(ctx context.Context, dbPath string, store storage.Storage, iss return nil, err } + // Purge deleted issues from DB based on deletions manifest + // Issues that are in the manifest but not in JSONL should be deleted from DB + if !opts.DryRun { + if err := purgeDeletedIssues(ctx, sqliteStore, dbPath, issues, result); err != nil { + // Non-fatal - just log warning + fmt.Fprintf(os.Stderr, "Warning: failed to purge deleted issues: %v\n", err) + } + } + // Checkpoint WAL to ensure data persistence and reduce WAL file size if err := sqliteStore.CheckpointWAL(ctx); err != nil { // Non-fatal - just log warning @@ -738,6 +751,73 @@ func importComments(ctx context.Context, sqliteStore *sqlite.SQLiteStorage, issu return nil } +// purgeDeletedIssues removes issues from the DB that are in the deletions manifest +// but not in the incoming JSONL. This enables deletion propagation across clones. +func purgeDeletedIssues(ctx context.Context, sqliteStore *sqlite.SQLiteStorage, dbPath string, jsonlIssues []*types.Issue, result *Result) error { + // Get deletions manifest path (same directory as database) + beadsDir := filepath.Dir(dbPath) + deletionsPath := deletions.DefaultPath(beadsDir) + + // Load deletions manifest (gracefully handles missing/empty file) + loadResult, err := deletions.LoadDeletions(deletionsPath) + if err != nil { + return fmt.Errorf("failed to load deletions manifest: %w", err) + } + + // Log any warnings from loading + for _, warning := range loadResult.Warnings { + fmt.Fprintf(os.Stderr, "Warning: %s\n", warning) + } + + // If no deletions, nothing to do + if len(loadResult.Records) == 0 { + return nil + } + + // Build set of IDs in the incoming JSONL for O(1) lookup + jsonlIDs := make(map[string]bool, len(jsonlIssues)) + for _, issue := range jsonlIssues { + jsonlIDs[issue.ID] = true + } + + // Get all DB issues + dbIssues, err := sqliteStore.SearchIssues(ctx, "", types.IssueFilter{}) + if err != nil { + return fmt.Errorf("failed to get DB issues: %w", err) + } + + // Find DB issues that: + // 1. Are NOT in the JSONL (not synced from remote) + // 2. ARE in the deletions manifest (were deleted elsewhere) + for _, dbIssue := range dbIssues { + if jsonlIDs[dbIssue.ID] { + // Issue is in JSONL, keep it + continue + } + + if del, found := loadResult.Records[dbIssue.ID]; found { + // Issue is in deletions manifest - purge it from DB + if err := sqliteStore.DeleteIssue(ctx, dbIssue.ID); err != nil { + fmt.Fprintf(os.Stderr, "Warning: failed to purge %s: %v\n", dbIssue.ID, err) + continue + } + + // Log the purge with metadata + fmt.Fprintf(os.Stderr, "Purged %s (deleted %s by %s", dbIssue.ID, del.Timestamp.Format("2006-01-02 15:04:05"), del.Actor) + if del.Reason != "" { + fmt.Fprintf(os.Stderr, ", reason: %s", del.Reason) + } + fmt.Fprintf(os.Stderr, ")\n") + + result.Purged++ + result.PurgedIDs = append(result.PurgedIDs, dbIssue.ID) + } + // If not in JSONL and not in deletions manifest, keep it (local work) + } + + return nil +} + // Helper functions func GetPrefixList(prefixes map[string]int) []string { diff --git a/internal/importer/purge_test.go b/internal/importer/purge_test.go new file mode 100644 index 00000000..73ee077f --- /dev/null +++ b/internal/importer/purge_test.go @@ -0,0 +1,233 @@ +package importer + +import ( + "context" + "os" + "path/filepath" + "testing" + "time" + + "github.com/steveyegge/beads/internal/deletions" + "github.com/steveyegge/beads/internal/storage/sqlite" + "github.com/steveyegge/beads/internal/types" +) + +// TestPurgeDeletedIssues tests that issues in the deletions manifest are purged during import +func TestPurgeDeletedIssues(t *testing.T) { + ctx := context.Background() + tmpDir := t.TempDir() + + // Create database + dbPath := filepath.Join(tmpDir, "beads.db") + store, err := sqlite.New(ctx, dbPath) + if err != nil { + t.Fatalf("failed to create database: %v", err) + } + defer store.Close() + + // Initialize prefix + if err := store.SetConfig(ctx, "issue_prefix", "test"); err != nil { + t.Fatalf("failed to set prefix: %v", err) + } + + // Create some issues in the database + issue1 := &types.Issue{ + ID: "test-abc", + Title: "Issue 1", + Status: types.StatusOpen, + Priority: 1, + IssueType: types.TypeTask, + } + issue2 := &types.Issue{ + ID: "test-def", + Title: "Issue 2", + Status: types.StatusOpen, + Priority: 1, + IssueType: types.TypeTask, + } + issue3 := &types.Issue{ + ID: "test-ghi", + Title: "Issue 3 (local work)", + Status: types.StatusOpen, + Priority: 1, + IssueType: types.TypeTask, + } + + for _, iss := range []*types.Issue{issue1, issue2, issue3} { + if err := store.CreateIssue(ctx, iss, "test"); err != nil { + t.Fatalf("failed to create issue %s: %v", iss.ID, err) + } + } + + // Create a deletions manifest with issue2 deleted + deletionsPath := deletions.DefaultPath(tmpDir) + delRecord := deletions.DeletionRecord{ + ID: "test-def", + Timestamp: time.Now().UTC(), + Actor: "test-user", + Reason: "test deletion", + } + if err := deletions.AppendDeletion(deletionsPath, delRecord); err != nil { + t.Fatalf("failed to create deletions manifest: %v", err) + } + + // Simulate import with only issue1 in the JSONL (issue2 was deleted, issue3 is local work) + jsonlIssues := []*types.Issue{issue1} + + result := &Result{ + IDMapping: make(map[string]string), + MismatchPrefixes: make(map[string]int), + } + + // Call purgeDeletedIssues + if err := purgeDeletedIssues(ctx, store, dbPath, jsonlIssues, result); err != nil { + t.Fatalf("purgeDeletedIssues failed: %v", err) + } + + // Verify issue2 was purged + if result.Purged != 1 { + t.Errorf("expected 1 purged issue, got %d", result.Purged) + } + if len(result.PurgedIDs) != 1 || result.PurgedIDs[0] != "test-def" { + t.Errorf("expected PurgedIDs to contain 'test-def', got %v", result.PurgedIDs) + } + + // Verify issue2 is gone from database + iss2, err := store.GetIssue(ctx, "test-def") + if err != nil { + t.Fatalf("GetIssue failed: %v", err) + } + if iss2 != nil { + t.Errorf("expected issue2 to be deleted, but it still exists") + } + + // Verify issue1 still exists (in JSONL) + iss1, err := store.GetIssue(ctx, "test-abc") + if err != nil { + t.Fatalf("GetIssue failed: %v", err) + } + if iss1 == nil { + t.Errorf("expected issue1 to still exist") + } + + // Verify issue3 still exists (local work, not in deletions manifest) + iss3, err := store.GetIssue(ctx, "test-ghi") + if err != nil { + t.Fatalf("GetIssue failed: %v", err) + } + if iss3 == nil { + t.Errorf("expected issue3 (local work) to still exist") + } +} + +// TestPurgeDeletedIssues_NoDeletionsManifest tests that import works without a deletions manifest +func TestPurgeDeletedIssues_NoDeletionsManifest(t *testing.T) { + ctx := context.Background() + tmpDir := t.TempDir() + + // Create database + dbPath := filepath.Join(tmpDir, "beads.db") + store, err := sqlite.New(ctx, dbPath) + if err != nil { + t.Fatalf("failed to create database: %v", err) + } + defer store.Close() + + // Initialize prefix + if err := store.SetConfig(ctx, "issue_prefix", "test"); err != nil { + t.Fatalf("failed to set prefix: %v", err) + } + + // Create an issue in the database + issue := &types.Issue{ + ID: "test-abc", + Title: "Issue 1", + Status: types.StatusOpen, + Priority: 1, + IssueType: types.TypeTask, + } + if err := store.CreateIssue(ctx, issue, "test"); err != nil { + t.Fatalf("failed to create issue: %v", err) + } + + // No deletions manifest exists + jsonlIssues := []*types.Issue{issue} + + result := &Result{ + IDMapping: make(map[string]string), + MismatchPrefixes: make(map[string]int), + } + + // Call purgeDeletedIssues - should succeed with no errors + if err := purgeDeletedIssues(ctx, store, dbPath, jsonlIssues, result); err != nil { + t.Fatalf("purgeDeletedIssues failed: %v", err) + } + + // Verify nothing was purged + if result.Purged != 0 { + t.Errorf("expected 0 purged issues, got %d", result.Purged) + } + + // Verify issue still exists + iss, err := store.GetIssue(ctx, "test-abc") + if err != nil { + t.Fatalf("GetIssue failed: %v", err) + } + if iss == nil { + t.Errorf("expected issue to still exist") + } +} + +// TestPurgeDeletedIssues_EmptyDeletionsManifest tests that import works with empty deletions manifest +func TestPurgeDeletedIssues_EmptyDeletionsManifest(t *testing.T) { + ctx := context.Background() + tmpDir := t.TempDir() + + // Create database + dbPath := filepath.Join(tmpDir, "beads.db") + store, err := sqlite.New(ctx, dbPath) + if err != nil { + t.Fatalf("failed to create database: %v", err) + } + defer store.Close() + + // Initialize prefix + if err := store.SetConfig(ctx, "issue_prefix", "test"); err != nil { + t.Fatalf("failed to set prefix: %v", err) + } + + // Create an issue in the database + issue := &types.Issue{ + ID: "test-abc", + Title: "Issue 1", + Status: types.StatusOpen, + Priority: 1, + IssueType: types.TypeTask, + } + if err := store.CreateIssue(ctx, issue, "test"); err != nil { + t.Fatalf("failed to create issue: %v", err) + } + + // Create empty deletions manifest + deletionsPath := deletions.DefaultPath(tmpDir) + if err := os.WriteFile(deletionsPath, []byte{}, 0644); err != nil { + t.Fatalf("failed to create empty deletions manifest: %v", err) + } + + jsonlIssues := []*types.Issue{issue} + + result := &Result{ + IDMapping: make(map[string]string), + MismatchPrefixes: make(map[string]int), + } + + // Call purgeDeletedIssues - should succeed with no errors + if err := purgeDeletedIssues(ctx, store, dbPath, jsonlIssues, result); err != nil { + t.Fatalf("purgeDeletedIssues failed: %v", err) + } + + // Verify nothing was purged + if result.Purged != 0 { + t.Errorf("expected 0 purged issues, got %d", result.Purged) + } +}