diff --git a/internal/storage/sqlite/ids.go b/internal/storage/sqlite/ids.go index 97644aea..0cdedd19 100644 --- a/internal/storage/sqlite/ids.go +++ b/internal/storage/sqlite/ids.go @@ -202,8 +202,13 @@ func EnsureIDs(ctx context.Context, conn *sql.Conn, prefix string, issues []*typ } } - // For hierarchical IDs (bd-a3f8e9.1), ensure parent exists - if strings.Contains(issues[i].ID, ".") { + // For hierarchical IDs (prefix-hash.child), ensure parent exists + // Only consider dots that appear AFTER the first hyphen (the prefix-hash separator) + // This avoids false positives when the prefix itself contains dots (e.g., "my.project-abc") + // See GH#508 + hyphenIdx := strings.Index(issues[i].ID, "-") + hasHierarchicalDot := hyphenIdx >= 0 && strings.Contains(issues[i].ID[hyphenIdx+1:], ".") + if hasHierarchicalDot { // Extract parent ID (everything before the last dot) lastDot := strings.LastIndex(issues[i].ID, ".") parentID := issues[i].ID[:lastDot] diff --git a/internal/storage/sqlite/migrations/016_orphan_detection.go b/internal/storage/sqlite/migrations/016_orphan_detection.go index d047f4a8..1f53c4e0 100644 --- a/internal/storage/sqlite/migrations/016_orphan_detection.go +++ b/internal/storage/sqlite/migrations/016_orphan_detection.go @@ -16,13 +16,17 @@ import ( // - Convert them to top-level issues by renaming them // - Restore the missing parent issues func MigrateOrphanDetection(db *sql.DB) error { - // Query for orphaned children using the pattern from the issue description: - // SELECT id FROM issues WHERE id LIKE '%.%' - // AND substr(id, 1, instr(id || '.', '.') - 1) NOT IN (SELECT id FROM issues) + // Query for orphaned children: + // - ID contains a dot (potential hierarchical structure) + // - The part before the first dot must contain a hyphen (GH#508) + // This ensures the dot is in the hash portion (e.g., "bd-abc.1"), not in the prefix + // (e.g., "my.project-abc123" where "my.project" is the prefix from directory name) + // - Parent (part before first dot) doesn't exist in database rows, err := db.Query(` SELECT id FROM issues WHERE id LIKE '%.%' + AND instr(substr(id, 1, instr(id, '.') - 1), '-') > 0 AND substr(id, 1, instr(id || '.', '.') - 1) NOT IN (SELECT id FROM issues) ORDER BY id `) diff --git a/internal/storage/sqlite/orphan_handling_test.go b/internal/storage/sqlite/orphan_handling_test.go index 6206739f..3ae2229c 100644 --- a/internal/storage/sqlite/orphan_handling_test.go +++ b/internal/storage/sqlite/orphan_handling_test.go @@ -235,6 +235,131 @@ func TestOrphanHandling_Config(t *testing.T) { } } +// 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()