Files
beads/internal/storage/sqlite/orphan_handling_test.go
Steve Yegge 39e09449cc fix: orphan detection false positive with dots in directory name
Only treat issue IDs as hierarchical (parent.child) when the dot appears
AFTER the first hyphen. This prevents false positives when the project
directory name contains a dot (e.g., "my.project-abc123" was incorrectly
being treated as having parent "my").

Fixes GH#508

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

Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
2025-12-16 01:06:14 -08:00

396 lines
11 KiB
Go

package sqlite
import (
"context"
"strings"
"testing"
"time"
"github.com/steveyegge/beads/internal/types"
)
// TestOrphanHandling_Strict tests that strict mode fails on missing parent
func TestOrphanHandling_Strict(t *testing.T) {
ctx := context.Background()
store, cleanup := setupTestDB(t)
defer cleanup()
if err := store.SetConfig(ctx, "issue_prefix", "test"); err != nil {
t.Fatalf("Failed to set prefix: %v", err)
}
// Try to create child without parent (strict mode)
child := &types.Issue{
ID: "test-abc.1", // Hierarchical child
Title: "Child issue",
Priority: 1,
IssueType: "task",
Status: "open",
}
err := store.CreateIssuesWithOptions(ctx, []*types.Issue{child}, "test", OrphanStrict)
if err == nil {
t.Fatal("Expected error in strict mode with missing parent")
}
if !strings.Contains(err.Error(), "parent") && !strings.Contains(err.Error(), "missing") {
t.Errorf("Expected error about missing parent, got: %v", err)
}
}
// TestOrphanHandling_Resurrect tests that resurrect mode auto-creates parent tombstones
func TestOrphanHandling_Resurrect(t *testing.T) {
ctx := context.Background()
store, cleanup := setupTestDB(t)
defer cleanup()
if err := store.SetConfig(ctx, "issue_prefix", "test"); err != nil {
t.Fatalf("Failed to set prefix: %v", err)
}
// Create a child with missing parent - resurrect mode should auto-create parent
child := &types.Issue{
ID: "test-abc.1", // Hierarchical child
Title: "Child issue",
Priority: 1,
IssueType: "task",
Status: "open",
}
// In resurrect mode, we need to provide the parent in the same batch
// This is because resurrect searches the batch for the parent
now := time.Now()
parent := &types.Issue{
ID: "test-abc",
Title: "Resurrected parent",
Priority: 4,
IssueType: "epic",
Status: "closed",
ClosedAt: &now,
}
// Import both together - resurrect logic is in EnsureIDs
err := store.CreateIssuesWithOptions(ctx, []*types.Issue{parent, child}, "test", OrphanResurrect)
if err != nil {
t.Fatalf("Resurrect mode should succeed: %v", err)
}
// Verify both parent and child exist
issues, err := store.SearchIssues(ctx, "", types.IssueFilter{})
if err != nil {
t.Fatalf("Failed to search issues: %v", err)
}
if len(issues) != 2 {
t.Fatalf("Expected 2 issues (parent + child), got %d", len(issues))
}
// Check parent was created as tombstone (closed, low priority)
var foundParent, foundChild bool
for _, issue := range issues {
if issue.ID == "test-abc" {
foundParent = true
if issue.Status != "closed" {
t.Errorf("Resurrected parent should be closed, got %s", issue.Status)
}
if issue.Priority != 4 {
t.Errorf("Resurrected parent should have priority 4, got %d", issue.Priority)
}
}
if issue.ID == "test-abc.1" {
foundChild = true
}
}
if !foundParent {
t.Error("Parent issue not found")
}
if !foundChild {
t.Error("Child issue not found")
}
}
// TestOrphanHandling_Skip tests that skip mode skips orphans with warning
func TestOrphanHandling_Skip(t *testing.T) {
ctx := context.Background()
store, cleanup := setupTestDB(t)
defer cleanup()
if err := store.SetConfig(ctx, "issue_prefix", "test"); err != nil {
t.Fatalf("Failed to set prefix: %v", err)
}
// Try to create child without parent (skip mode)
child := &types.Issue{
ID: "test-abc.1", // Hierarchical child
Title: "Child issue",
Priority: 1,
IssueType: "task",
Status: "open",
}
// In skip mode, operation should succeed but child should not be created
err := store.CreateIssuesWithOptions(ctx, []*types.Issue{child}, "test", OrphanSkip)
// Skip mode should not error, but also should not create the child
// Note: Current implementation may still error - need to check implementation
// For now, we'll verify the child wasn't created
issues, searchErr := store.SearchIssues(ctx, "", types.IssueFilter{})
if searchErr != nil {
t.Fatalf("Failed to search issues: %v", searchErr)
}
// Child should have been skipped
for _, issue := range issues {
if issue.ID == "test-abc.1" {
t.Errorf("Child issue should have been skipped but was created: %+v", issue)
}
}
// If skip mode is working correctly, we expect either:
// 1. No error and empty database (child skipped)
// 2. Error mentioning skip/warning
if err != nil && !strings.Contains(err.Error(), "skip") && !strings.Contains(err.Error(), "missing parent") {
t.Logf("Skip mode error (may be expected): %v", err)
}
}
// TestOrphanHandling_Allow tests that allow mode imports orphans without validation
func TestOrphanHandling_Allow(t *testing.T) {
ctx := context.Background()
store, cleanup := setupTestDB(t)
defer cleanup()
if err := store.SetConfig(ctx, "issue_prefix", "test"); err != nil {
t.Fatalf("Failed to set prefix: %v", err)
}
// Create child without parent (allow mode) - should succeed
child := &types.Issue{
ID: "test-abc.1", // Hierarchical child
Title: "Orphaned child",
Priority: 1,
IssueType: "task",
Status: "open",
}
err := store.CreateIssuesWithOptions(ctx, []*types.Issue{child}, "test", OrphanAllow)
if err != nil {
t.Fatalf("Allow mode should succeed even with missing parent: %v", err)
}
// Verify child was created
issues, err := store.SearchIssues(ctx, "", types.IssueFilter{})
if err != nil {
t.Fatalf("Failed to search issues: %v", err)
}
if len(issues) != 1 {
t.Fatalf("Expected 1 issue (orphaned child), got %d", len(issues))
}
if issues[0].ID != "test-abc.1" {
t.Errorf("Expected child ID test-abc.1, got %s", issues[0].ID)
}
}
// TestOrphanHandling_Config tests reading orphan handling from config
func TestOrphanHandling_Config(t *testing.T) {
ctx := context.Background()
store, cleanup := setupTestDB(t)
defer cleanup()
tests := []struct {
name string
configValue string
expectedMode OrphanHandling
}{
{"strict mode", "strict", OrphanStrict},
{"resurrect mode", "resurrect", OrphanResurrect},
{"skip mode", "skip", OrphanSkip},
{"allow mode", "allow", OrphanAllow},
{"empty defaults to allow", "", OrphanAllow},
{"invalid defaults to allow", "invalid-mode", OrphanAllow},
}
for _, tt := range tests {
t.Run(tt.name, func(t *testing.T) {
if tt.configValue != "" {
if err := store.SetConfig(ctx, "import.orphan_handling", tt.configValue); err != nil {
t.Fatalf("Failed to set config: %v", err)
}
} else {
// Delete config to test empty case
if err := store.DeleteConfig(ctx, "import.orphan_handling"); err != nil {
t.Fatalf("Failed to delete config: %v", err)
}
}
mode := store.GetOrphanHandling(ctx)
if mode != tt.expectedMode {
t.Errorf("Expected mode %s, got %s", tt.expectedMode, mode)
}
})
}
}
// TestOrphanHandling_PrefixWithDots tests that prefixes containing dots don't trigger
// false positives in hierarchical ID detection (GH#508)
func TestOrphanHandling_PrefixWithDots(t *testing.T) {
ctx := context.Background()
store, cleanup := setupTestDB(t)
defer cleanup()
// Use a prefix with dots (simulating a directory name like "my.project")
if err := store.SetConfig(ctx, "issue_prefix", "my.project"); err != nil {
t.Fatalf("Failed to set prefix: %v", err)
}
// Create a top-level issue with a dotted prefix
// This should NOT be treated as a hierarchical child
issue := &types.Issue{
ID: "my.project-abc123",
Title: "Top-level issue with dotted prefix",
Priority: 1,
IssueType: "task",
Status: "open",
}
// In strict mode, this should succeed because it's not a hierarchical ID
// (the dot is in the prefix, not after the hyphen)
err := store.CreateIssuesWithOptions(ctx, []*types.Issue{issue}, "my.project", OrphanStrict)
if err != nil {
t.Fatalf("Expected success for non-hierarchical ID with dotted prefix, got: %v", err)
}
// Verify the issue was created
issues, err := store.SearchIssues(ctx, "", types.IssueFilter{})
if err != nil {
t.Fatalf("Failed to search issues: %v", err)
}
if len(issues) != 1 {
t.Fatalf("Expected 1 issue, got %d", len(issues))
}
if issues[0].ID != "my.project-abc123" {
t.Errorf("Expected ID my.project-abc123, got %s", issues[0].ID)
}
}
// TestOrphanHandling_PrefixWithDotsAndChild tests hierarchical children with dotted prefixes
func TestOrphanHandling_PrefixWithDotsAndChild(t *testing.T) {
ctx := context.Background()
store, cleanup := setupTestDB(t)
defer cleanup()
// Use a prefix with dots
if err := store.SetConfig(ctx, "issue_prefix", "my.project"); err != nil {
t.Fatalf("Failed to set prefix: %v", err)
}
// First create the parent
parent := &types.Issue{
ID: "my.project-abc123",
Title: "Parent issue",
Priority: 1,
IssueType: "epic",
Status: "open",
}
err := store.CreateIssuesWithOptions(ctx, []*types.Issue{parent}, "my.project", OrphanStrict)
if err != nil {
t.Fatalf("Failed to create parent: %v", err)
}
// Now create a child - this IS hierarchical (dot after hyphen)
child := &types.Issue{
ID: "my.project-abc123.1",
Title: "Child issue",
Priority: 1,
IssueType: "task",
Status: "open",
}
err = store.CreateIssuesWithOptions(ctx, []*types.Issue{child}, "my.project", OrphanStrict)
if err != nil {
t.Fatalf("Expected success for child with existing parent, got: %v", err)
}
// Verify both were created
issues, err := store.SearchIssues(ctx, "", types.IssueFilter{})
if err != nil {
t.Fatalf("Failed to search issues: %v", err)
}
if len(issues) != 2 {
t.Fatalf("Expected 2 issues, got %d", len(issues))
}
}
// TestOrphanHandling_PrefixWithDotsOrphanChild tests that orphan detection works correctly
// with dotted prefixes - should detect orphan when parent doesn't exist
func TestOrphanHandling_PrefixWithDotsOrphanChild(t *testing.T) {
ctx := context.Background()
store, cleanup := setupTestDB(t)
defer cleanup()
// Use a prefix with dots
if err := store.SetConfig(ctx, "issue_prefix", "my.project"); err != nil {
t.Fatalf("Failed to set prefix: %v", err)
}
// Try to create a child without the parent - should fail in strict mode
child := &types.Issue{
ID: "my.project-abc123.1", // This IS hierarchical (dot after hyphen)
Title: "Orphan child",
Priority: 1,
IssueType: "task",
Status: "open",
}
err := store.CreateIssuesWithOptions(ctx, []*types.Issue{child}, "my.project", OrphanStrict)
if err == nil {
t.Fatal("Expected error for orphan child in strict mode")
}
if !strings.Contains(err.Error(), "parent") {
t.Errorf("Expected error about missing parent, got: %v", err)
}
}
// TestOrphanHandling_NonHierarchical tests that non-hierarchical IDs work in all modes
func TestOrphanHandling_NonHierarchical(t *testing.T) {
ctx := context.Background()
store, cleanup := setupTestDB(t)
defer cleanup()
if err := store.SetConfig(ctx, "issue_prefix", "test"); err != nil {
t.Fatalf("Failed to set prefix: %v", err)
}
// Non-hierarchical issues should work in all modes
issue := &types.Issue{
ID: "test-xyz", // Non-hierarchical (no dot)
Title: "Regular issue",
Priority: 1,
IssueType: "task",
Status: "open",
}
modes := []OrphanHandling{OrphanStrict, OrphanResurrect, OrphanSkip, OrphanAllow}
for _, mode := range modes {
t.Run(string(mode), func(t *testing.T) {
// Use unique ID for each mode
testIssue := *issue
testIssue.ID = "test-" + string(mode)
err := store.CreateIssuesWithOptions(ctx, []*types.Issue{&testIssue}, "test", mode)
if err != nil {
t.Errorf("Non-hierarchical issue should succeed in %s mode: %v", mode, err)
}
})
}
}