diff --git a/internal/issue/blocking.go b/internal/issue/blocking.go index 8da2e44b..641d5a9e 100644 --- a/internal/issue/blocking.go +++ b/internal/issue/blocking.go @@ -445,15 +445,25 @@ func (o *subtreeOverlay) effectiveBlockedBy(iss *Issue) []string { // buildSubtreeOverlay builds subtrees from parent fields, collects // external blockers per display root, and marks all descendants. // -// A "display root" is the shallowest open/deferred ancestor with children. -// When an ancestor is in_progress or in_review it is already claimed work, so -// we drill past it to surface the actual next-step descendants. +// A "display root" is the shallowest open ancestor with children that has not +// yet been claimed. We drill past a node (recursing into its children to +// surface the actual next-step frontier) when: +// - it is in_progress or in_review — already claimed work; or +// - it is open but a descendant is already in_progress/in_review — work has +// begun inside the subtree even though the node itself was never started, +// so the node is effectively underway and is suppressed from its own +// listing; or +// - it is deferred with an unexpired defer date — the node is hidden from +// ready, but its open, unblocked children should still surface as the +// frontier rather than being collapsed away with the hidden parent. func (s *Store) buildSubtreeOverlay() *subtreeOverlay { overlay := &subtreeOverlay{ descendants: make(map[string]bool), externalBlockers: make(map[string][]string), } + now := s.Now() + // Load all non-closed issues var allIDs []string for _, status := range []string{"open", "in_progress", "in_review", "deferred"} { @@ -473,9 +483,26 @@ func (s *Store) buildSubtreeOverlay() *subtreeOverlay { } } + // subtreeHasClaimedWork reports whether any descendant of id (not id + // itself) is in_progress or in_review. + var subtreeHasClaimedWork func(id string) bool + subtreeHasClaimedWork = func(id string) bool { + for _, c := range children[id] { + ci := issues[c] + if ci == nil { + continue + } + if ci.Status == "in_progress" || ci.Status == "in_review" { + return true + } + if subtreeHasClaimedWork(c) { + return true + } + } + return false + } + // Walk down from each top-level structural root to find display roots. - // Claimed nodes (in_progress/in_review) are not display roots themselves — - // we recurse into their children to surface the actual ready frontier. var displayRoots []string var walk func(id string) walk = func(id string) { @@ -483,13 +510,33 @@ func (s *Store) buildSubtreeOverlay() *subtreeOverlay { if iss == nil { return } + // Claimed work: drill past to surface the frontier beneath it. if iss.Status == "in_progress" || iss.Status == "in_review" { for _, c := range children[id] { walk(c) } return } + // Future-deferred node: hidden from ready, but its children should + // still surface. Drill past without listing it (the deferred loop in + // Ready will not re-add it while its deferral is unexpired). + if iss.Status == "deferred" && !IsDeferralExpired(iss.DeferUntil, now) { + for _, c := range children[id] { + walk(c) + } + return + } if len(children[id]) > 0 { + // An open node with claimed work inside it is effectively + // underway. Drill past it to surface the live frontier, and + // suppress the node from its own listing. + if subtreeHasClaimedWork(id) { + overlay.descendants[id] = true + for _, c := range children[id] { + walk(c) + } + return + } displayRoots = append(displayRoots, id) } } diff --git a/internal/issue/blocking_test.go b/internal/issue/blocking_test.go index a1827f06..421b5612 100644 --- a/internal/issue/blocking_test.go +++ b/internal/issue/blocking_test.go @@ -1528,6 +1528,150 @@ func TestReadyStopsAtOpenIntermediateNode(t *testing.T) { } } +// When a child is started directly (without first starting the parent epic), +// the epic stays open but work has begun inside it. Ready() should drill past +// the open epic and surface the open sibling frontier — not collapse to just +// the epic and hide the actionable siblings. +func TestReadyDrillsPastOpenRootWithClaimedChild(t *testing.T) { + env := testutil.NewEnv(t) + defer env.Cleanup() + + epic, _ := env.Store.Create("Epic", issue.CreateOpts{Type: "epic"}) + childB, _ := env.Store.Create("Child B", issue.CreateOpts{Parent: epic.ID}) + childC, _ := env.Store.Create("Child C", issue.CreateOpts{Parent: epic.ID}) + env.CommitIntent("setup") + + // Start child B directly; the epic is left open. + if _, err := env.Store.Start(childB.ID, ""); err != nil { + t.Fatalf("Start child: %v", err) + } + env.CommitIntent("start " + childB.ID) + + ready, err := env.Store.Ready() + if err != nil { + t.Fatalf("Ready: %v", err) + } + ids := make(map[string]bool) + for _, r := range ready { + ids[r.ID] = true + } + if !ids[childC.ID] { + t.Errorf("open sibling C should surface as the frontier, got %v", ids) + } + if ids[epic.ID] { + t.Errorf("open epic should be suppressed (work claimed inside it), got %v", ids) + } + if ids[childB.ID] { + t.Errorf("in_progress child B should not appear in ready, got %v", ids) + } +} + +// An open epic with no claimed work inside it keeps the original behavior: +// the epic is the display root and its children stay hidden until it's started. +func TestReadyOpenRootNoClaimedWorkStillCollapses(t *testing.T) { + env := testutil.NewEnv(t) + defer env.Cleanup() + + epic, _ := env.Store.Create("Epic", issue.CreateOpts{Type: "epic"}) + childB, _ := env.Store.Create("Child B", issue.CreateOpts{Parent: epic.ID}) + childC, _ := env.Store.Create("Child C", issue.CreateOpts{Parent: epic.ID}) + env.CommitIntent("setup") + + ready, err := env.Store.Ready() + if err != nil { + t.Fatalf("Ready: %v", err) + } + ids := make(map[string]bool) + for _, r := range ready { + ids[r.ID] = true + } + if !ids[epic.ID] { + t.Errorf("epic should be the display root, got %v", ids) + } + if ids[childB.ID] || ids[childC.ID] { + t.Errorf("children should stay hidden under an unstarted epic, got %v", ids) + } +} + +// A future-deferred epic is hidden from ready, but its open, unblocked children +// should still surface as the frontier rather than being suppressed along with +// the hidden parent. +func TestReadyDeferredParentSurfacesOpenChildren(t *testing.T) { + env := testutil.NewEnv(t) + defer env.Cleanup() + + epic, _ := env.Store.Create("Epic", issue.CreateOpts{Type: "epic"}) + childE, _ := env.Store.Create("Child E", issue.CreateOpts{Parent: epic.ID}) + childF, _ := env.Store.Create("Child F", issue.CreateOpts{Parent: epic.ID}) + env.CommitIntent("setup") + + // Defer the epic far into the future; children remain open and unblocked. + deferred := "deferred" + deferDate := "2099-01-01" + if _, err := env.Store.Update(epic.ID, issue.UpdateOpts{Status: &deferred, DeferUntil: &deferDate}); err != nil { + t.Fatalf("defer epic: %v", err) + } + env.CommitIntent("defer " + epic.ID) + + ready, err := env.Store.Ready() + if err != nil { + t.Fatalf("Ready: %v", err) + } + ids := make(map[string]bool) + for _, r := range ready { + ids[r.ID] = true + } + if !ids[childE.ID] || !ids[childF.ID] { + t.Errorf("open children of a deferred epic should surface, got %v", ids) + } + if ids[epic.ID] { + t.Errorf("future-deferred epic should not appear in ready, got %v", ids) + } +} + +// When work is claimed two levels down (a grandchild started while both the +// epic and the intermediate node stay open), Ready() must still recognize the +// subtree as underway: suppress the open epic AND the open intermediate, and +// surface the open sibling as the frontier. This exercises the recursive +// descent in subtreeHasClaimedWork. +func TestReadyDrillsPastOpenRootWithTransitivelyClaimedChild(t *testing.T) { + env := testutil.NewEnv(t) + defer env.Cleanup() + + epic, _ := env.Store.Create("Epic", issue.CreateOpts{Type: "epic"}) + mid, _ := env.Store.Create("Mid", issue.CreateOpts{Parent: epic.ID}) + leaf, _ := env.Store.Create("Leaf", issue.CreateOpts{Parent: mid.ID}) + sib, _ := env.Store.Create("Sib", issue.CreateOpts{Parent: epic.ID}) + env.CommitIntent("setup") + + // Start the grandchild directly; epic and mid are left open. + if _, err := env.Store.Start(leaf.ID, ""); err != nil { + t.Fatalf("Start leaf: %v", err) + } + env.CommitIntent("start " + leaf.ID) + + ready, err := env.Store.Ready() + if err != nil { + t.Fatalf("Ready: %v", err) + } + ids := make(map[string]bool) + for _, r := range ready { + ids[r.ID] = true + } + if !ids[sib.ID] { + t.Errorf("open sibling should surface as the frontier, got %v", ids) + } + if ids[epic.ID] { + t.Errorf("open epic should be suppressed (transitively claimed work), got %v", ids) + } + if ids[mid.ID] { + t.Errorf("open intermediate node should be suppressed (claimed child below it), got %v", ids) + } + if ids[leaf.ID] { + t.Errorf("in_progress leaf should not appear in ready, got %v", ids) + } +} + // External blockers should bubble to the display root (which can be a deeper // node than the top-level epic when the epic is in_progress). func TestReadyExternalBlockerBubblesToDisplayRoot(t *testing.T) {