e816e91ecb
This commit addresses the remaining P2 tasks from bd-ar2 code review follow-up: ## Completed Tasks ### bd-ar2.4: Improve parent chain resurrection - Modified `tryResurrectParentWithConn()` to recursively resurrect ancestor chain - When resurrecting bd-root.1.2, now also resurrects bd-root.1 if missing - Handles deeply nested hierarchies where intermediate parents are deleted - All resurrection tests pass including new edge cases ### bd-ar2.5: Add error handling guidance - Documented metadata update failure strategy in `updateExportMetadata()` - Explained trade-off: warnings vs errors (safe, prevents data loss) - Added user-facing message: "Next export may require running 'bd import' first" - Clarifies that worst case is requiring import before next export ### bd-ar2.6: Document transaction boundaries - Added comprehensive documentation for atomicity trade-offs - Explained crash scenarios and recovery (bd import) - Documented decision to defer defensive checks (Option 4) until needed - No code changes - current approach is acceptable for now ### bd-ar2.12: Add metadata key validation - Added keySuffix validation in `updateExportMetadata()` and `hasJSONLChanged()` - Prevents ':' separator in keySuffix to avoid malformed metadata keys - Documented metadata key format in function comments - Single-repo: "last_import_hash", multi-repo: "last_import_hash:<repo_key>" ### bd-ar2.7: Add edge case tests for GetNextChildID resurrection - TestGetNextChildID_ResurrectParent_NotInJSONL: parent not in history - TestGetNextChildID_ResurrectParent_NoJSONL: missing JSONL file - TestGetNextChildID_ResurrectParent_MalformedJSONL: invalid JSON lines - TestGetNextChildID_ResurrectParentChain: deeply nested missing parents - All tests pass, resurrection is robust against edge cases ## Files Changed - cmd/bd/daemon_sync.go: Metadata validation, error handling docs - cmd/bd/integrity.go: Added strings import, keySuffix validation - internal/storage/sqlite/hash_ids.go: Improved resurrection comments - internal/storage/sqlite/resurrection.go: Recursive ancestor resurrection - internal/storage/sqlite/child_id_test.go: Added 4 new edge case tests ## Testing All export, sync, metadata, and resurrection tests pass. Edge cases properly handled: missing JSONL, malformed JSON, deep nesting. ## Remaining Tasks - bd-ar2.8 (P3): Additional export metadata edge case tests (deferred) 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <noreply@anthropic.com>
418 lines
14 KiB
Go
418 lines
14 KiB
Go
package sqlite
|
|
|
|
import (
|
|
"context"
|
|
"os"
|
|
"testing"
|
|
|
|
"github.com/steveyegge/beads/internal/types"
|
|
)
|
|
|
|
func TestGetNextChildID(t *testing.T) {
|
|
tmpFile := t.TempDir() + "/test.db"
|
|
defer os.Remove(tmpFile)
|
|
store := newTestStore(t, tmpFile)
|
|
defer store.Close()
|
|
ctx := context.Background()
|
|
|
|
// Create a parent issue with hash ID
|
|
parent := &types.Issue{
|
|
ID: "bd-a3f8e9",
|
|
Title: "Parent Epic",
|
|
Description: "Parent issue",
|
|
Status: types.StatusOpen,
|
|
Priority: 1,
|
|
IssueType: types.TypeEpic,
|
|
}
|
|
if err := store.CreateIssue(ctx, parent, "test"); err != nil {
|
|
t.Fatalf("failed to create parent: %v", err)
|
|
}
|
|
|
|
// Test: Generate first child ID
|
|
childID1, err := store.GetNextChildID(ctx, parent.ID)
|
|
if err != nil {
|
|
t.Fatalf("GetNextChildID failed: %v", err)
|
|
}
|
|
expectedID1 := "bd-a3f8e9.1"
|
|
if childID1 != expectedID1 {
|
|
t.Errorf("expected %s, got %s", expectedID1, childID1)
|
|
}
|
|
|
|
// Test: Generate second child ID (sequential)
|
|
childID2, err := store.GetNextChildID(ctx, parent.ID)
|
|
if err != nil {
|
|
t.Fatalf("GetNextChildID failed: %v", err)
|
|
}
|
|
expectedID2 := "bd-a3f8e9.2"
|
|
if childID2 != expectedID2 {
|
|
t.Errorf("expected %s, got %s", expectedID2, childID2)
|
|
}
|
|
|
|
// Create the first child and test nested hierarchy
|
|
child1 := &types.Issue{
|
|
ID: childID1,
|
|
Title: "Child Task 1",
|
|
Description: "First child",
|
|
Status: types.StatusOpen,
|
|
Priority: 1,
|
|
IssueType: types.TypeTask,
|
|
}
|
|
if err := store.CreateIssue(ctx, child1, "test"); err != nil {
|
|
t.Fatalf("failed to create child: %v", err)
|
|
}
|
|
|
|
// Test: Generate nested child (depth 2)
|
|
nestedID1, err := store.GetNextChildID(ctx, childID1)
|
|
if err != nil {
|
|
t.Fatalf("GetNextChildID failed for nested: %v", err)
|
|
}
|
|
expectedNested1 := "bd-a3f8e9.1.1"
|
|
if nestedID1 != expectedNested1 {
|
|
t.Errorf("expected %s, got %s", expectedNested1, nestedID1)
|
|
}
|
|
|
|
// Create the nested child
|
|
nested1 := &types.Issue{
|
|
ID: nestedID1,
|
|
Title: "Nested Task",
|
|
Description: "Nested child",
|
|
Status: types.StatusOpen,
|
|
Priority: 1,
|
|
IssueType: types.TypeTask,
|
|
}
|
|
if err := store.CreateIssue(ctx, nested1, "test"); err != nil {
|
|
t.Fatalf("failed to create nested child: %v", err)
|
|
}
|
|
|
|
// Test: Generate third level (depth 3, maximum)
|
|
deepID1, err := store.GetNextChildID(ctx, nestedID1)
|
|
if err != nil {
|
|
t.Fatalf("GetNextChildID failed for depth 3: %v", err)
|
|
}
|
|
expectedDeep1 := "bd-a3f8e9.1.1.1"
|
|
if deepID1 != expectedDeep1 {
|
|
t.Errorf("expected %s, got %s", expectedDeep1, deepID1)
|
|
}
|
|
|
|
// Create the deep child
|
|
deep1 := &types.Issue{
|
|
ID: deepID1,
|
|
Title: "Deep Task",
|
|
Description: "Third level",
|
|
Status: types.StatusOpen,
|
|
Priority: 1,
|
|
IssueType: types.TypeTask,
|
|
}
|
|
if err := store.CreateIssue(ctx, deep1, "test"); err != nil {
|
|
t.Fatalf("failed to create deep child: %v", err)
|
|
}
|
|
|
|
// Test: Attempt to create fourth level (should fail)
|
|
_, err = store.GetNextChildID(ctx, deepID1)
|
|
if err == nil {
|
|
t.Errorf("expected error for depth 4, got nil")
|
|
}
|
|
if err != nil && err.Error() != "maximum hierarchy depth (3) exceeded for parent bd-a3f8e9.1.1.1" {
|
|
t.Errorf("unexpected error message: %v", err)
|
|
}
|
|
}
|
|
|
|
func TestGetNextChildID_ParentNotExists(t *testing.T) {
|
|
tmpFile := t.TempDir() + "/test.db"
|
|
defer os.Remove(tmpFile)
|
|
store := newTestStore(t, tmpFile)
|
|
defer store.Close()
|
|
ctx := context.Background()
|
|
|
|
// Test: Attempt to get child ID for non-existent parent
|
|
_, err := store.GetNextChildID(ctx, "bd-nonexistent")
|
|
if err == nil {
|
|
t.Errorf("expected error for non-existent parent, got nil")
|
|
}
|
|
// With resurrection feature (bd-dvd fix), error message includes JSONL history check
|
|
expectedErr := "parent issue bd-nonexistent does not exist and could not be resurrected from JSONL history"
|
|
if err != nil && err.Error() != expectedErr {
|
|
t.Errorf("unexpected error message: got %q, want %q", err.Error(), expectedErr)
|
|
}
|
|
}
|
|
|
|
func TestCreateIssue_HierarchicalID(t *testing.T) {
|
|
tmpFile := t.TempDir() + "/test.db"
|
|
defer os.Remove(tmpFile)
|
|
store := newTestStore(t, tmpFile)
|
|
defer store.Close()
|
|
ctx := context.Background()
|
|
|
|
// Create parent
|
|
parent := &types.Issue{
|
|
ID: "bd-parent1",
|
|
Title: "Parent",
|
|
Description: "Parent issue",
|
|
Status: types.StatusOpen,
|
|
Priority: 1,
|
|
IssueType: types.TypeEpic,
|
|
}
|
|
if err := store.CreateIssue(ctx, parent, "test"); err != nil {
|
|
t.Fatalf("failed to create parent: %v", err)
|
|
}
|
|
|
|
// Test: Create child with explicit hierarchical ID
|
|
child := &types.Issue{
|
|
ID: "bd-parent1.1",
|
|
Title: "Child",
|
|
Description: "Child issue",
|
|
Status: types.StatusOpen,
|
|
Priority: 1,
|
|
IssueType: types.TypeTask,
|
|
}
|
|
if err := store.CreateIssue(ctx, child, "test"); err != nil {
|
|
t.Fatalf("failed to create child: %v", err)
|
|
}
|
|
|
|
// Verify child was created
|
|
retrieved, err := store.GetIssue(ctx, child.ID)
|
|
if err != nil {
|
|
t.Fatalf("failed to retrieve child: %v", err)
|
|
}
|
|
if retrieved.ID != child.ID {
|
|
t.Errorf("expected ID %s, got %s", child.ID, retrieved.ID)
|
|
}
|
|
}
|
|
|
|
func TestCreateIssue_HierarchicalID_ParentNotExists(t *testing.T) {
|
|
tmpFile := t.TempDir() + "/test.db"
|
|
defer os.Remove(tmpFile)
|
|
store := newTestStore(t, tmpFile)
|
|
defer store.Close()
|
|
ctx := context.Background()
|
|
|
|
// Test: Attempt to create child without parent
|
|
child := &types.Issue{
|
|
ID: "bd-nonexistent.1",
|
|
Title: "Child",
|
|
Description: "Child issue",
|
|
Status: types.StatusOpen,
|
|
Priority: 1,
|
|
IssueType: types.TypeTask,
|
|
}
|
|
err := store.CreateIssue(ctx, child, "test")
|
|
if err == nil {
|
|
t.Errorf("expected error for child without parent, got nil")
|
|
}
|
|
// With resurrection feature, error message includes JSONL history check
|
|
expectedErr := "parent issue bd-nonexistent does not exist and could not be resurrected from JSONL history"
|
|
if err != nil && err.Error() != expectedErr {
|
|
t.Errorf("unexpected error message: got %q, want %q", err.Error(), expectedErr)
|
|
}
|
|
}
|
|
|
|
func TestGetNextChildID_ResurrectParent(t *testing.T) {
|
|
tmpDir := t.TempDir()
|
|
tmpFile := tmpDir + "/test.db"
|
|
defer os.Remove(tmpFile)
|
|
store := newTestStore(t, tmpFile)
|
|
defer store.Close()
|
|
ctx := context.Background()
|
|
|
|
// Create parent issue
|
|
parent := &types.Issue{
|
|
ID: "bd-test123",
|
|
ContentHash: "abc123",
|
|
Title: "Parent Issue",
|
|
Description: "Parent to be resurrected",
|
|
Status: types.StatusOpen,
|
|
Priority: 1,
|
|
IssueType: types.TypeEpic,
|
|
}
|
|
if err := store.CreateIssue(ctx, parent, "test"); err != nil {
|
|
t.Fatalf("failed to create parent: %v", err)
|
|
}
|
|
|
|
// Delete the parent from database (simulating deletion)
|
|
if err := store.DeleteIssue(ctx, parent.ID); err != nil {
|
|
t.Fatalf("failed to delete parent: %v", err)
|
|
}
|
|
|
|
// Create JSONL file with the deleted parent (simulating JSONL history)
|
|
// Note: This requires the JSONL to be in .beads/issues.jsonl relative to dbPath
|
|
// The resurrection logic looks for issues.jsonl in the same directory as the database
|
|
beadsDir := tmpDir
|
|
jsonlPath := beadsDir + "/issues.jsonl"
|
|
|
|
// Write parent to JSONL
|
|
jsonlFile, err := os.Create(jsonlPath)
|
|
if err != nil {
|
|
t.Fatalf("failed to create JSONL file: %v", err)
|
|
}
|
|
parentJSON := `{"id":"bd-test123","content_hash":"abc123","title":"Parent Issue","description":"Parent to be resurrected","status":"open","priority":1,"type":"epic","created_at":"2025-01-01T00:00:00Z","updated_at":"2025-01-01T00:00:00Z"}`
|
|
if _, err := jsonlFile.WriteString(parentJSON + "\n"); err != nil {
|
|
jsonlFile.Close()
|
|
t.Fatalf("failed to write to JSONL: %v", err)
|
|
}
|
|
jsonlFile.Close()
|
|
|
|
// Now attempt to get next child ID - should resurrect parent
|
|
childID, err := store.GetNextChildID(ctx, parent.ID)
|
|
if err != nil {
|
|
t.Fatalf("GetNextChildID should have resurrected parent, but got error: %v", err)
|
|
}
|
|
|
|
expectedID := "bd-test123.1"
|
|
if childID != expectedID {
|
|
t.Errorf("expected child ID %s, got %s", expectedID, childID)
|
|
}
|
|
|
|
// Verify parent was resurrected as tombstone
|
|
resurrectedParent, err := store.GetIssue(ctx, parent.ID)
|
|
if err != nil {
|
|
t.Fatalf("failed to get resurrected parent: %v", err)
|
|
}
|
|
if resurrectedParent.Status != types.StatusClosed {
|
|
t.Errorf("expected resurrected parent to be closed, got %s", resurrectedParent.Status)
|
|
}
|
|
if resurrectedParent.Title != "Parent Issue" {
|
|
t.Errorf("expected resurrected parent title to be preserved, got %s", resurrectedParent.Title)
|
|
}
|
|
}
|
|
|
|
// TestGetNextChildID_ResurrectParent_NotInJSONL tests resurrection when parent doesn't exist in JSONL (bd-ar2.7)
|
|
func TestGetNextChildID_ResurrectParent_NotInJSONL(t *testing.T) {
|
|
tmpDir := t.TempDir()
|
|
tmpFile := tmpDir + "/test.db"
|
|
defer os.Remove(tmpFile)
|
|
store := newTestStore(t, tmpFile)
|
|
defer store.Close()
|
|
ctx := context.Background()
|
|
|
|
// Create empty JSONL file (parent not in history)
|
|
jsonlPath := tmpDir + "/issues.jsonl"
|
|
if err := os.WriteFile(jsonlPath, []byte(""), 0600); err != nil {
|
|
t.Fatalf("failed to create JSONL file: %v", err)
|
|
}
|
|
|
|
// Attempt to get child ID for non-existent parent not in JSONL
|
|
_, err := store.GetNextChildID(ctx, "bd-notfound")
|
|
if err == nil {
|
|
t.Errorf("expected error for parent not in JSONL, got nil")
|
|
}
|
|
expectedErr := "parent issue bd-notfound does not exist and could not be resurrected from JSONL history"
|
|
if err != nil && err.Error() != expectedErr {
|
|
t.Errorf("unexpected error: got %q, want %q", err.Error(), expectedErr)
|
|
}
|
|
}
|
|
|
|
// TestGetNextChildID_ResurrectParent_NoJSONL tests resurrection when JSONL file doesn't exist (bd-ar2.7)
|
|
func TestGetNextChildID_ResurrectParent_NoJSONL(t *testing.T) {
|
|
tmpDir := t.TempDir()
|
|
tmpFile := tmpDir + "/test.db"
|
|
defer os.Remove(tmpFile)
|
|
store := newTestStore(t, tmpFile)
|
|
defer store.Close()
|
|
ctx := context.Background()
|
|
|
|
// No JSONL file created
|
|
// Attempt to get child ID for non-existent parent
|
|
_, err := store.GetNextChildID(ctx, "bd-missing")
|
|
if err == nil {
|
|
t.Errorf("expected error for parent with no JSONL, got nil")
|
|
}
|
|
expectedErr := "parent issue bd-missing does not exist and could not be resurrected from JSONL history"
|
|
if err != nil && err.Error() != expectedErr {
|
|
t.Errorf("unexpected error: got %q, want %q", err.Error(), expectedErr)
|
|
}
|
|
}
|
|
|
|
// TestGetNextChildID_ResurrectParent_MalformedJSONL tests resurrection with invalid JSON lines (bd-ar2.7)
|
|
func TestGetNextChildID_ResurrectParent_MalformedJSONL(t *testing.T) {
|
|
tmpDir := t.TempDir()
|
|
tmpFile := tmpDir + "/test.db"
|
|
defer os.Remove(tmpFile)
|
|
store := newTestStore(t, tmpFile)
|
|
defer store.Close()
|
|
ctx := context.Background()
|
|
|
|
// Create JSONL with malformed lines and one valid parent
|
|
jsonlPath := tmpDir + "/issues.jsonl"
|
|
jsonlContent := `{invalid json
|
|
{"id":"bd-test456","content_hash":"def456","title":"Valid Parent","description":"Should be found","status":"open","priority":1,"type":"epic","created_at":"2025-01-01T00:00:00Z","updated_at":"2025-01-01T00:00:00Z"}
|
|
this is not json either
|
|
`
|
|
if err := os.WriteFile(jsonlPath, []byte(jsonlContent), 0600); err != nil {
|
|
t.Fatalf("failed to create JSONL file: %v", err)
|
|
}
|
|
|
|
// Should successfully resurrect despite malformed lines
|
|
childID, err := store.GetNextChildID(ctx, "bd-test456")
|
|
if err != nil {
|
|
t.Fatalf("GetNextChildID should skip malformed lines and resurrect valid parent, got error: %v", err)
|
|
}
|
|
|
|
expectedID := "bd-test456.1"
|
|
if childID != expectedID {
|
|
t.Errorf("expected child ID %s, got %s", expectedID, childID)
|
|
}
|
|
}
|
|
|
|
// TestGetNextChildID_ResurrectParentChain tests resurrection of deeply nested missing parents (bd-ar2.7)
|
|
func TestGetNextChildID_ResurrectParentChain(t *testing.T) {
|
|
tmpDir := t.TempDir()
|
|
tmpFile := tmpDir + "/test.db"
|
|
defer os.Remove(tmpFile)
|
|
store := newTestStore(t, tmpFile)
|
|
defer store.Close()
|
|
ctx := context.Background()
|
|
|
|
// Create root parent only
|
|
root := &types.Issue{
|
|
ID: "bd-root",
|
|
ContentHash: "root123",
|
|
Title: "Root Issue",
|
|
Description: "Root",
|
|
Status: types.StatusOpen,
|
|
Priority: 1,
|
|
IssueType: types.TypeEpic,
|
|
}
|
|
if err := store.CreateIssue(ctx, root, "test"); err != nil {
|
|
t.Fatalf("failed to create root: %v", err)
|
|
}
|
|
|
|
// Create JSONL with intermediate parents that are deleted
|
|
jsonlPath := tmpDir + "/issues.jsonl"
|
|
jsonlContent := `{"id":"bd-root","content_hash":"root123","title":"Root Issue","description":"Root","status":"open","priority":1,"type":"epic","created_at":"2025-01-01T00:00:00Z","updated_at":"2025-01-01T00:00:00Z"}
|
|
{"id":"bd-root.1","content_hash":"l1abc","title":"Level 1","description":"First level","status":"open","priority":1,"type":"task","created_at":"2025-01-01T00:00:00Z","updated_at":"2025-01-01T00:00:00Z"}
|
|
{"id":"bd-root.1.2","content_hash":"l2abc","title":"Level 2","description":"Second level","status":"open","priority":1,"type":"task","created_at":"2025-01-01T00:00:00Z","updated_at":"2025-01-01T00:00:00Z"}
|
|
`
|
|
if err := os.WriteFile(jsonlPath, []byte(jsonlContent), 0600); err != nil {
|
|
t.Fatalf("failed to create JSONL file: %v", err)
|
|
}
|
|
|
|
// Try to create child of bd-root.1.2 (which doesn't exist in DB, but its parent bd-root.1 also doesn't exist)
|
|
// With TryResurrectParentChain (bd-ar2.4), this should work
|
|
childID, err := store.GetNextChildID(ctx, "bd-root.1.2")
|
|
if err != nil {
|
|
t.Fatalf("GetNextChildID should resurrect entire parent chain, got error: %v", err)
|
|
}
|
|
|
|
expectedID := "bd-root.1.2.1"
|
|
if childID != expectedID {
|
|
t.Errorf("expected child ID %s, got %s", expectedID, childID)
|
|
}
|
|
|
|
// Verify both intermediate parents were resurrected
|
|
parent1, err := store.GetIssue(ctx, "bd-root.1")
|
|
if err != nil {
|
|
t.Fatalf("bd-root.1 should have been resurrected: %v", err)
|
|
}
|
|
if parent1.Status != types.StatusClosed {
|
|
t.Errorf("expected resurrected parent to be closed, got %s", parent1.Status)
|
|
}
|
|
|
|
parent2, err := store.GetIssue(ctx, "bd-root.1.2")
|
|
if err != nil {
|
|
t.Fatalf("bd-root.1.2 should have been resurrected: %v", err)
|
|
}
|
|
if parent2.Status != types.StatusClosed {
|
|
t.Errorf("expected resurrected parent to be closed, got %s", parent2.Status)
|
|
}
|
|
}
|