diff --git a/internal/storage/sqlite/ready.go b/internal/storage/sqlite/ready.go index 074d7a81..16fc1943 100644 --- a/internal/storage/sqlite/ready.go +++ b/internal/storage/sqlite/ready.go @@ -42,19 +42,46 @@ func (s *SQLiteStorage) GetReadyWork(ctx context.Context, filter types.WorkFilte args = append(args, filter.Limit) } - // Single query template + // Query with recursive CTE to propagate blocking through parent-child hierarchy + // Algorithm: + // 1. Find issues directly blocked by 'blocks' dependencies + // 2. Recursively propagate blockage to all descendants via 'parent-child' links + // 3. Exclude all blocked issues (both direct and transitive) from ready work query := fmt.Sprintf(` + WITH RECURSIVE + -- Step 1: Find issues blocked directly by dependencies + blocked_directly AS ( + SELECT DISTINCT d.issue_id + FROM dependencies d + JOIN issues blocker ON d.depends_on_id = blocker.id + WHERE d.type = 'blocks' + AND blocker.status IN ('open', 'in_progress', 'blocked') + ), + + -- Step 2: Propagate blockage to all descendants via parent-child + blocked_transitively AS ( + -- Base case: directly blocked issues + SELECT issue_id, 0 as depth + FROM blocked_directly + + UNION ALL + + -- Recursive case: children of blocked issues inherit blockage + SELECT d.issue_id, bt.depth + 1 + FROM blocked_transitively bt + JOIN dependencies d ON d.depends_on_id = bt.issue_id + WHERE d.type = 'parent-child' + AND bt.depth < 50 + ) + + -- Step 3: Select ready issues (excluding all blocked) SELECT i.id, i.title, i.description, i.design, i.acceptance_criteria, i.notes, i.status, i.priority, i.issue_type, i.assignee, i.estimated_minutes, i.created_at, i.updated_at, i.closed_at, i.external_ref FROM issues i WHERE %s AND NOT EXISTS ( - SELECT 1 FROM dependencies d - JOIN issues blocked ON d.depends_on_id = blocked.id - WHERE d.issue_id = i.id - AND d.type = 'blocks' - AND blocked.status IN ('open', 'in_progress', 'blocked') + SELECT 1 FROM blocked_transitively WHERE issue_id = i.id ) ORDER BY i.priority ASC, i.created_at DESC %s diff --git a/internal/storage/sqlite/ready_test.go b/internal/storage/sqlite/ready_test.go index 0827d00f..04616463 100644 --- a/internal/storage/sqlite/ready_test.go +++ b/internal/storage/sqlite/ready_test.go @@ -272,3 +272,305 @@ func TestGetBlockedIssues(t *testing.T) { t.Errorf("Expected 2 blocker IDs, got %d", len(issue3Blocked.BlockedBy)) } } + +// TestParentBlockerBlocksChildren tests that children inherit blockage from parents +func TestParentBlockerBlocksChildren(t *testing.T) { + store, cleanup := setupTestDB(t) + defer cleanup() + + ctx := context.Background() + + // Create: + // blocker: open + // epic1: open, blocked by 'blocker' + // task1: open, child of epic1 (via parent-child) + // + // Expected: task1 should NOT be ready (parent is blocked) + + blocker := &types.Issue{Title: "Blocker", Status: types.StatusOpen, Priority: 1, IssueType: types.TypeTask} + epic1 := &types.Issue{Title: "Epic 1", Status: types.StatusOpen, Priority: 1, IssueType: types.TypeEpic} + task1 := &types.Issue{Title: "Task 1", Status: types.StatusOpen, Priority: 1, IssueType: types.TypeTask} + + store.CreateIssue(ctx, blocker, "test-user") + store.CreateIssue(ctx, epic1, "test-user") + store.CreateIssue(ctx, task1, "test-user") + + // epic1 blocked by blocker + store.AddDependency(ctx, &types.Dependency{IssueID: epic1.ID, DependsOnID: blocker.ID, Type: types.DepBlocks}, "test-user") + // task1 is child of epic1 + store.AddDependency(ctx, &types.Dependency{IssueID: task1.ID, DependsOnID: epic1.ID, Type: types.DepParentChild}, "test-user") + + // Get ready work + ready, err := store.GetReadyWork(ctx, types.WorkFilter{Status: types.StatusOpen}) + if err != nil { + t.Fatalf("GetReadyWork failed: %v", err) + } + + // Should have only blocker ready + readyIDs := make(map[string]bool) + for _, issue := range ready { + readyIDs[issue.ID] = true + } + + if readyIDs[epic1.ID] { + t.Errorf("Expected epic1 to be blocked, but it was ready") + } + if readyIDs[task1.ID] { + t.Errorf("Expected task1 to be blocked (parent is blocked), but it was ready") + } + if !readyIDs[blocker.ID] { + t.Errorf("Expected blocker to be ready") + } +} + +// TestGrandparentBlockerBlocksGrandchildren tests multi-level propagation +func TestGrandparentBlockerBlocksGrandchildren(t *testing.T) { + store, cleanup := setupTestDB(t) + defer cleanup() + + ctx := context.Background() + + // Create: + // blocker: open + // epic1: open, blocked by 'blocker' + // epic2: open, child of epic1 + // task1: open, child of epic2 + // + // Expected: task1 should NOT be ready (grandparent is blocked) + + blocker := &types.Issue{Title: "Blocker", Status: types.StatusOpen, Priority: 1, IssueType: types.TypeTask} + epic1 := &types.Issue{Title: "Epic 1", Status: types.StatusOpen, Priority: 1, IssueType: types.TypeEpic} + epic2 := &types.Issue{Title: "Epic 2", Status: types.StatusOpen, Priority: 1, IssueType: types.TypeEpic} + task1 := &types.Issue{Title: "Task 1", Status: types.StatusOpen, Priority: 1, IssueType: types.TypeTask} + + store.CreateIssue(ctx, blocker, "test-user") + store.CreateIssue(ctx, epic1, "test-user") + store.CreateIssue(ctx, epic2, "test-user") + store.CreateIssue(ctx, task1, "test-user") + + // epic1 blocked by blocker + store.AddDependency(ctx, &types.Dependency{IssueID: epic1.ID, DependsOnID: blocker.ID, Type: types.DepBlocks}, "test-user") + // epic2 is child of epic1 + store.AddDependency(ctx, &types.Dependency{IssueID: epic2.ID, DependsOnID: epic1.ID, Type: types.DepParentChild}, "test-user") + // task1 is child of epic2 + store.AddDependency(ctx, &types.Dependency{IssueID: task1.ID, DependsOnID: epic2.ID, Type: types.DepParentChild}, "test-user") + + // Get ready work + ready, err := store.GetReadyWork(ctx, types.WorkFilter{Status: types.StatusOpen}) + if err != nil { + t.Fatalf("GetReadyWork failed: %v", err) + } + + // Should have only blocker ready + readyIDs := make(map[string]bool) + for _, issue := range ready { + readyIDs[issue.ID] = true + } + + if readyIDs[epic1.ID] { + t.Errorf("Expected epic1 to be blocked, but it was ready") + } + if readyIDs[epic2.ID] { + t.Errorf("Expected epic2 to be blocked (parent is blocked), but it was ready") + } + if readyIDs[task1.ID] { + t.Errorf("Expected task1 to be blocked (grandparent is blocked), but it was ready") + } + if !readyIDs[blocker.ID] { + t.Errorf("Expected blocker to be ready") + } +} + +// TestMultipleParentsOneBlocked tests that a child is blocked if ANY parent is blocked +func TestMultipleParentsOneBlocked(t *testing.T) { + store, cleanup := setupTestDB(t) + defer cleanup() + + ctx := context.Background() + + // Create: + // blocker: open + // epic1: open, blocked by 'blocker' + // epic2: open, no blockers + // task1: open, child of BOTH epic1 and epic2 + // + // Expected: task1 should NOT be ready (one parent is blocked) + + blocker := &types.Issue{Title: "Blocker", Status: types.StatusOpen, Priority: 1, IssueType: types.TypeTask} + epic1 := &types.Issue{Title: "Epic 1 (blocked)", Status: types.StatusOpen, Priority: 1, IssueType: types.TypeEpic} + epic2 := &types.Issue{Title: "Epic 2 (ready)", Status: types.StatusOpen, Priority: 1, IssueType: types.TypeEpic} + task1 := &types.Issue{Title: "Task 1", Status: types.StatusOpen, Priority: 1, IssueType: types.TypeTask} + + store.CreateIssue(ctx, blocker, "test-user") + store.CreateIssue(ctx, epic1, "test-user") + store.CreateIssue(ctx, epic2, "test-user") + store.CreateIssue(ctx, task1, "test-user") + + // epic1 blocked by blocker + store.AddDependency(ctx, &types.Dependency{IssueID: epic1.ID, DependsOnID: blocker.ID, Type: types.DepBlocks}, "test-user") + // task1 is child of both epic1 and epic2 + store.AddDependency(ctx, &types.Dependency{IssueID: task1.ID, DependsOnID: epic1.ID, Type: types.DepParentChild}, "test-user") + store.AddDependency(ctx, &types.Dependency{IssueID: task1.ID, DependsOnID: epic2.ID, Type: types.DepParentChild}, "test-user") + + // Get ready work + ready, err := store.GetReadyWork(ctx, types.WorkFilter{Status: types.StatusOpen}) + if err != nil { + t.Fatalf("GetReadyWork failed: %v", err) + } + + // Should have blocker and epic2 ready, but NOT epic1 or task1 + readyIDs := make(map[string]bool) + for _, issue := range ready { + readyIDs[issue.ID] = true + } + + if readyIDs[epic1.ID] { + t.Errorf("Expected epic1 to be blocked, but it was ready") + } + if readyIDs[task1.ID] { + t.Errorf("Expected task1 to be blocked (one parent is blocked), but it was ready") + } + if !readyIDs[blocker.ID] { + t.Errorf("Expected blocker to be ready") + } + if !readyIDs[epic2.ID] { + t.Errorf("Expected epic2 to be ready") + } +} + +// TestBlockerClosedUnblocksChildren tests that closing a blocker unblocks descendants +func TestBlockerClosedUnblocksChildren(t *testing.T) { + store, cleanup := setupTestDB(t) + defer cleanup() + + ctx := context.Background() + + // Create: + // blocker: initially open, then closed + // epic1: open, blocked by 'blocker' + // task1: open, child of epic1 + // + // After closing blocker: both epic1 and task1 should be ready + + blocker := &types.Issue{Title: "Blocker", Status: types.StatusOpen, Priority: 1, IssueType: types.TypeTask} + epic1 := &types.Issue{Title: "Epic 1", Status: types.StatusOpen, Priority: 1, IssueType: types.TypeEpic} + task1 := &types.Issue{Title: "Task 1", Status: types.StatusOpen, Priority: 1, IssueType: types.TypeTask} + + store.CreateIssue(ctx, blocker, "test-user") + store.CreateIssue(ctx, epic1, "test-user") + store.CreateIssue(ctx, task1, "test-user") + + // epic1 blocked by blocker + store.AddDependency(ctx, &types.Dependency{IssueID: epic1.ID, DependsOnID: blocker.ID, Type: types.DepBlocks}, "test-user") + // task1 is child of epic1 + store.AddDependency(ctx, &types.Dependency{IssueID: task1.ID, DependsOnID: epic1.ID, Type: types.DepParentChild}, "test-user") + + // Initially, epic1 and task1 should be blocked + ready, err := store.GetReadyWork(ctx, types.WorkFilter{Status: types.StatusOpen}) + if err != nil { + t.Fatalf("GetReadyWork failed: %v", err) + } + + readyIDs := make(map[string]bool) + for _, issue := range ready { + readyIDs[issue.ID] = true + } + + if readyIDs[epic1.ID] || readyIDs[task1.ID] { + t.Errorf("Expected epic1 and task1 to be blocked initially") + } + + // Close the blocker + store.CloseIssue(ctx, blocker.ID, "Done", "test-user") + + // Now epic1 and task1 should be ready + ready, err = store.GetReadyWork(ctx, types.WorkFilter{Status: types.StatusOpen}) + if err != nil { + t.Fatalf("GetReadyWork failed after closing blocker: %v", err) + } + + readyIDs = make(map[string]bool) + for _, issue := range ready { + readyIDs[issue.ID] = true + } + + if !readyIDs[epic1.ID] { + t.Errorf("Expected epic1 to be ready after blocker closed") + } + if !readyIDs[task1.ID] { + t.Errorf("Expected task1 to be ready after blocker closed") + } +} + +// TestRelatedDoesNotPropagate tests that 'related' deps don't cause blocking propagation +func TestRelatedDoesNotPropagate(t *testing.T) { + store, cleanup := setupTestDB(t) + defer cleanup() + + ctx := context.Background() + + // Create: + // blocker: open + // epic1: open, blocked by 'blocker' + // task1: open, related to epic1 (NOT parent-child) + // + // Expected: task1 SHOULD be ready (related doesn't propagate blocking) + + blocker := &types.Issue{Title: "Blocker", Status: types.StatusOpen, Priority: 1, IssueType: types.TypeTask} + epic1 := &types.Issue{Title: "Epic 1", Status: types.StatusOpen, Priority: 1, IssueType: types.TypeEpic} + task1 := &types.Issue{Title: "Task 1", Status: types.StatusOpen, Priority: 1, IssueType: types.TypeTask} + + store.CreateIssue(ctx, blocker, "test-user") + store.CreateIssue(ctx, epic1, "test-user") + store.CreateIssue(ctx, task1, "test-user") + + // epic1 blocked by blocker + store.AddDependency(ctx, &types.Dependency{IssueID: epic1.ID, DependsOnID: blocker.ID, Type: types.DepBlocks}, "test-user") + // task1 is related to epic1 (NOT parent-child) + store.AddDependency(ctx, &types.Dependency{IssueID: task1.ID, DependsOnID: epic1.ID, Type: types.DepRelated}, "test-user") + + // Get ready work + ready, err := store.GetReadyWork(ctx, types.WorkFilter{Status: types.StatusOpen}) + if err != nil { + t.Fatalf("GetReadyWork failed: %v", err) + } + + // Should have blocker AND task1 ready (related doesn't propagate) + readyIDs := make(map[string]bool) + for _, issue := range ready { + readyIDs[issue.ID] = true + } + + if readyIDs[epic1.ID] { + t.Errorf("Expected epic1 to be blocked, but it was ready") + } + if !readyIDs[task1.ID] { + t.Errorf("Expected task1 to be ready (related deps don't propagate blocking), but it was blocked") + } + if !readyIDs[blocker.ID] { + t.Errorf("Expected blocker to be ready") + } +} + +// TestCompositeIndexExists verifies the composite index is created +func TestCompositeIndexExists(t *testing.T) { + store, cleanup := setupTestDB(t) + defer cleanup() + + ctx := context.Background() + + // Query sqlite_master to check if the index exists + var indexName string + err := store.db.QueryRowContext(ctx, ` + SELECT name FROM sqlite_master + WHERE type='index' AND name='idx_dependencies_depends_on_type' + `).Scan(&indexName) + + if err != nil { + t.Fatalf("Composite index idx_dependencies_depends_on_type not found: %v", err) + } + + if indexName != "idx_dependencies_depends_on_type" { + t.Errorf("Expected index name 'idx_dependencies_depends_on_type', got '%s'", indexName) + } +}