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>
This commit is contained in:
Steve Yegge
2025-12-16 01:05:55 -08:00
parent 2c86404d65
commit 39e09449cc
3 changed files with 139 additions and 5 deletions

View File

@@ -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]

View File

@@ -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
`)

View File

@@ -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()