fix: Propagate blocking through parent-child hierarchy [fixes #19]
When an epic is blocked, all its children should also be considered blocked in the ready work calculation. Previously, only direct blocking dependencies were checked, allowing children of blocked epics to appear as ready work. Implementation: - Use recursive CTE to propagate blocking from parents to descendants - Only 'parent-child' dependencies propagate blocking (not 'related') - Changed NOT IN to NOT EXISTS for better NULL safety and performance - Added depth limit of 50 to prevent pathological cases Test coverage: - TestParentBlockerBlocksChildren: Basic parent→child propagation - TestGrandparentBlockerBlocksGrandchildren: Multi-level depth - TestMultipleParentsOneBlocked: Child blocked if ANY parent blocked - TestBlockerClosedUnblocksChildren: Dynamic unblocking works - TestRelatedDoesNotPropagate: Only parent-child propagates Closes: https://github.com/steveyegge/beads/issues/19 Resolves: bd-58 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <noreply@anthropic.com>
This commit is contained in:
Steve Yegge
@@ -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
|
||||
|
||||
@@ -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)
|
||||
}
|
||||
}
|
||||
|
||||
Reference in New Issue
Block a user