feat: display external deps in bd dep tree (bd-vks2, bd-mv6h, bd-d9mu)
External dependencies (external:project:capability) are now visible in the dependency tree output. Previously they were invisible because the recursive CTE only JOINed against the issues table. Changes: - GetDependencyTree now fetches external deps and adds them as synthetic leaf nodes with resolution status (satisfied/blocked) - formatTreeNode displays external deps with special formatting - Added helper parseExternalRefParts for parsing external refs Test coverage added for: - External deps appearing in dependency tree - Cycle detection ignoring external refs - CheckExternalDep when target has no .beads directory - Various invalid external ref format variations Closes: bd-vks2, bd-mv6h, bd-d9mu
This commit is contained in:
@@ -639,6 +639,21 @@ func (r *treeRenderer) renderNode(node *types.TreeNode, children map[string][]*t
|
|||||||
|
|
||||||
// formatTreeNode formats a single tree node with status, ready indicator, etc.
|
// formatTreeNode formats a single tree node with status, ready indicator, etc.
|
||||||
func formatTreeNode(node *types.TreeNode) string {
|
func formatTreeNode(node *types.TreeNode) string {
|
||||||
|
// Handle external dependencies specially (bd-vks2)
|
||||||
|
if IsExternalRef(node.ID) {
|
||||||
|
// External deps use their title directly which includes the status indicator
|
||||||
|
var idStr string
|
||||||
|
switch node.Status {
|
||||||
|
case types.StatusClosed:
|
||||||
|
idStr = ui.StatusClosedStyle.Render(node.Title)
|
||||||
|
case types.StatusBlocked:
|
||||||
|
idStr = ui.StatusBlockedStyle.Render(node.Title)
|
||||||
|
default:
|
||||||
|
idStr = node.Title
|
||||||
|
}
|
||||||
|
return fmt.Sprintf("%s (external)", idStr)
|
||||||
|
}
|
||||||
|
|
||||||
// Color the ID based on status
|
// Color the ID based on status
|
||||||
var idStr string
|
var idStr string
|
||||||
switch node.Status {
|
switch node.Status {
|
||||||
|
|||||||
@@ -625,9 +625,97 @@ func (s *SQLiteStorage) GetDependencyTree(ctx context.Context, issueID string, m
|
|||||||
nodes = append(nodes, &node)
|
nodes = append(nodes, &node)
|
||||||
}
|
}
|
||||||
|
|
||||||
|
// Fetch external dependencies for all issues in the tree (bd-vks2)
|
||||||
|
// External deps like "external:project:capability" don't exist in the issues
|
||||||
|
// table, so the recursive CTE above doesn't find them. We add them as
|
||||||
|
// synthetic leaf nodes here.
|
||||||
|
if len(nodes) > 0 && !reverse {
|
||||||
|
// Collect all issue IDs in the tree
|
||||||
|
issueIDs := make([]string, len(nodes))
|
||||||
|
depthByID := make(map[string]int)
|
||||||
|
for i, n := range nodes {
|
||||||
|
issueIDs[i] = n.ID
|
||||||
|
depthByID[n.ID] = n.Depth
|
||||||
|
}
|
||||||
|
|
||||||
|
// Query for external dependencies
|
||||||
|
externalDeps, err := s.getExternalDepsForIssues(ctx, issueIDs)
|
||||||
|
if err != nil {
|
||||||
|
// Non-fatal: just skip external deps if query fails
|
||||||
|
_ = err
|
||||||
|
} else {
|
||||||
|
// Create synthetic TreeNode for each external dep
|
||||||
|
for parentID, extRefs := range externalDeps {
|
||||||
|
parentDepth, ok := depthByID[parentID]
|
||||||
|
if !ok {
|
||||||
|
continue
|
||||||
|
}
|
||||||
|
// Skip if we've exceeded maxDepth
|
||||||
|
if parentDepth >= maxDepth {
|
||||||
|
continue
|
||||||
|
}
|
||||||
|
|
||||||
|
for _, ref := range extRefs {
|
||||||
|
// Parse external ref for display
|
||||||
|
_, capability := parseExternalRefParts(ref)
|
||||||
|
if capability == "" {
|
||||||
|
capability = ref // fallback to full ref
|
||||||
|
}
|
||||||
|
|
||||||
|
// Check resolution status
|
||||||
|
status := CheckExternalDep(ctx, ref)
|
||||||
|
var nodeStatus types.Status
|
||||||
|
var title string
|
||||||
|
if status.Satisfied {
|
||||||
|
nodeStatus = types.StatusClosed
|
||||||
|
title = fmt.Sprintf("✓ %s", capability)
|
||||||
|
} else {
|
||||||
|
nodeStatus = types.StatusBlocked
|
||||||
|
title = fmt.Sprintf("⏳ %s", capability)
|
||||||
|
}
|
||||||
|
|
||||||
|
extNode := &types.TreeNode{
|
||||||
|
Issue: types.Issue{
|
||||||
|
ID: ref,
|
||||||
|
Title: title,
|
||||||
|
Status: nodeStatus,
|
||||||
|
Priority: 0, // External deps don't have priority
|
||||||
|
IssueType: types.TypeTask,
|
||||||
|
},
|
||||||
|
Depth: parentDepth + 1,
|
||||||
|
ParentID: parentID,
|
||||||
|
}
|
||||||
|
|
||||||
|
// Apply deduplication if needed
|
||||||
|
if !showAllPaths {
|
||||||
|
if _, exists := seen[ref]; exists {
|
||||||
|
continue
|
||||||
|
}
|
||||||
|
seen[ref] = extNode.Depth
|
||||||
|
}
|
||||||
|
|
||||||
|
nodes = append(nodes, extNode)
|
||||||
|
}
|
||||||
|
}
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
return nodes, nil
|
return nodes, nil
|
||||||
}
|
}
|
||||||
|
|
||||||
|
// parseExternalRefParts parses "external:project:capability" and returns (project, capability).
|
||||||
|
// Returns empty strings if the format is invalid.
|
||||||
|
func parseExternalRefParts(ref string) (project, capability string) {
|
||||||
|
if !strings.HasPrefix(ref, "external:") {
|
||||||
|
return "", ""
|
||||||
|
}
|
||||||
|
parts := strings.SplitN(ref, ":", 3)
|
||||||
|
if len(parts) != 3 {
|
||||||
|
return "", ""
|
||||||
|
}
|
||||||
|
return parts[1], parts[2]
|
||||||
|
}
|
||||||
|
|
||||||
// DetectCycles finds circular dependencies and returns the actual cycle paths
|
// DetectCycles finds circular dependencies and returns the actual cycle paths
|
||||||
// Note: relates-to dependencies are excluded because they are intentionally bidirectional
|
// Note: relates-to dependencies are excluded because they are intentionally bidirectional
|
||||||
// ("see also" relationships) and do not represent problematic cycles.
|
// ("see also" relationships) and do not represent problematic cycles.
|
||||||
|
|||||||
@@ -1657,3 +1657,161 @@ func TestRemoveDependencyExternal(t *testing.T) {
|
|||||||
t.Errorf("Expected 0 dependencies after removal, got %d", len(deps))
|
t.Errorf("Expected 0 dependencies after removal, got %d", len(deps))
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
|
// TestGetDependencyTreeExternalDeps verifies that external dependencies
|
||||||
|
// appear in the dependency tree as synthetic leaf nodes (bd-vks2).
|
||||||
|
func TestGetDependencyTreeExternalDeps(t *testing.T) {
|
||||||
|
store, cleanup := setupTestDB(t)
|
||||||
|
defer cleanup()
|
||||||
|
|
||||||
|
ctx := context.Background()
|
||||||
|
|
||||||
|
// Create a root issue
|
||||||
|
root := &types.Issue{
|
||||||
|
Title: "Root issue",
|
||||||
|
Status: types.StatusOpen,
|
||||||
|
Priority: 1,
|
||||||
|
IssueType: types.TypeTask,
|
||||||
|
}
|
||||||
|
if err := store.CreateIssue(ctx, root, "test-user"); err != nil {
|
||||||
|
t.Fatalf("CreateIssue failed: %v", err)
|
||||||
|
}
|
||||||
|
|
||||||
|
// Create a blocking local issue
|
||||||
|
blocker := &types.Issue{
|
||||||
|
Title: "Local blocker",
|
||||||
|
Status: types.StatusOpen,
|
||||||
|
Priority: 1,
|
||||||
|
IssueType: types.TypeTask,
|
||||||
|
}
|
||||||
|
if err := store.CreateIssue(ctx, blocker, "test-user"); err != nil {
|
||||||
|
t.Fatalf("CreateIssue failed: %v", err)
|
||||||
|
}
|
||||||
|
|
||||||
|
// Add local dependency
|
||||||
|
localDep := &types.Dependency{
|
||||||
|
IssueID: root.ID,
|
||||||
|
DependsOnID: blocker.ID,
|
||||||
|
Type: types.DepBlocks,
|
||||||
|
}
|
||||||
|
if err := store.AddDependency(ctx, localDep, "test-user"); err != nil {
|
||||||
|
t.Fatalf("AddDependency (local) failed: %v", err)
|
||||||
|
}
|
||||||
|
|
||||||
|
// Add external dependency to root
|
||||||
|
extRef := "external:test-project:test-capability"
|
||||||
|
extDep := &types.Dependency{
|
||||||
|
IssueID: root.ID,
|
||||||
|
DependsOnID: extRef,
|
||||||
|
Type: types.DepBlocks,
|
||||||
|
}
|
||||||
|
if err := store.AddDependency(ctx, extDep, "test-user"); err != nil {
|
||||||
|
t.Fatalf("AddDependency (external) failed: %v", err)
|
||||||
|
}
|
||||||
|
|
||||||
|
// Get dependency tree
|
||||||
|
tree, err := store.GetDependencyTree(ctx, root.ID, 10, false, false)
|
||||||
|
if err != nil {
|
||||||
|
t.Fatalf("GetDependencyTree failed: %v", err)
|
||||||
|
}
|
||||||
|
|
||||||
|
// Should have 3 nodes: root, local blocker, and external dep
|
||||||
|
if len(tree) != 3 {
|
||||||
|
t.Errorf("Expected 3 nodes in tree (root, local, external), got %d", len(tree))
|
||||||
|
for _, node := range tree {
|
||||||
|
t.Logf("Node: id=%s title=%s depth=%d", node.ID, node.Title, node.Depth)
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
|
// Find the external dep node
|
||||||
|
var extNode *types.TreeNode
|
||||||
|
for _, node := range tree {
|
||||||
|
if node.ID == extRef {
|
||||||
|
extNode = node
|
||||||
|
break
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
|
if extNode == nil {
|
||||||
|
t.Fatal("External dependency not found in tree")
|
||||||
|
}
|
||||||
|
|
||||||
|
// Verify external node properties
|
||||||
|
if extNode.Depth != 1 {
|
||||||
|
t.Errorf("Expected external dep at depth 1, got %d", extNode.Depth)
|
||||||
|
}
|
||||||
|
if extNode.ParentID != root.ID {
|
||||||
|
t.Errorf("Expected external dep parent to be root, got %s", extNode.ParentID)
|
||||||
|
}
|
||||||
|
// External deps should show blocked status when not configured
|
||||||
|
if extNode.Status != types.StatusBlocked {
|
||||||
|
t.Errorf("Expected external dep status to be blocked (not configured), got %s", extNode.Status)
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
|
// TestCycleDetectionWithExternalRefs verifies that external dependencies
|
||||||
|
// don't participate in cycle detection (they can't form cycles with local issues).
|
||||||
|
func TestCycleDetectionWithExternalRefs(t *testing.T) {
|
||||||
|
store, cleanup := setupTestDB(t)
|
||||||
|
defer cleanup()
|
||||||
|
|
||||||
|
ctx := context.Background()
|
||||||
|
|
||||||
|
// Create two issues
|
||||||
|
issueA := &types.Issue{
|
||||||
|
Title: "Issue A",
|
||||||
|
Status: types.StatusOpen,
|
||||||
|
Priority: 1,
|
||||||
|
IssueType: types.TypeTask,
|
||||||
|
}
|
||||||
|
if err := store.CreateIssue(ctx, issueA, "test-user"); err != nil {
|
||||||
|
t.Fatalf("CreateIssue A failed: %v", err)
|
||||||
|
}
|
||||||
|
|
||||||
|
issueB := &types.Issue{
|
||||||
|
Title: "Issue B",
|
||||||
|
Status: types.StatusOpen,
|
||||||
|
Priority: 1,
|
||||||
|
IssueType: types.TypeTask,
|
||||||
|
}
|
||||||
|
if err := store.CreateIssue(ctx, issueB, "test-user"); err != nil {
|
||||||
|
t.Fatalf("CreateIssue B failed: %v", err)
|
||||||
|
}
|
||||||
|
|
||||||
|
// A depends on B
|
||||||
|
if err := store.AddDependency(ctx, &types.Dependency{
|
||||||
|
IssueID: issueA.ID,
|
||||||
|
DependsOnID: issueB.ID,
|
||||||
|
Type: types.DepBlocks,
|
||||||
|
}, "test-user"); err != nil {
|
||||||
|
t.Fatalf("AddDependency A->B failed: %v", err)
|
||||||
|
}
|
||||||
|
|
||||||
|
// B depends on external ref (should succeed - external refs don't form cycles)
|
||||||
|
extRef := "external:project:capability"
|
||||||
|
if err := store.AddDependency(ctx, &types.Dependency{
|
||||||
|
IssueID: issueB.ID,
|
||||||
|
DependsOnID: extRef,
|
||||||
|
Type: types.DepBlocks,
|
||||||
|
}, "test-user"); err != nil {
|
||||||
|
t.Fatalf("AddDependency B->external failed: %v", err)
|
||||||
|
}
|
||||||
|
|
||||||
|
// A depends on same external ref (should also succeed - no cycle with external)
|
||||||
|
if err := store.AddDependency(ctx, &types.Dependency{
|
||||||
|
IssueID: issueA.ID,
|
||||||
|
DependsOnID: extRef,
|
||||||
|
Type: types.DepBlocks,
|
||||||
|
}, "test-user"); err != nil {
|
||||||
|
t.Fatalf("AddDependency A->external failed: %v", err)
|
||||||
|
}
|
||||||
|
|
||||||
|
// Verify DetectCycles doesn't find any cycles
|
||||||
|
cycles, err := store.DetectCycles(ctx)
|
||||||
|
if err != nil {
|
||||||
|
t.Fatalf("DetectCycles failed: %v", err)
|
||||||
|
}
|
||||||
|
if len(cycles) != 0 {
|
||||||
|
t.Errorf("Expected no cycles with external deps, got %d", len(cycles))
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|||||||
@@ -1652,3 +1652,115 @@ func TestGetBlockedIssuesPartialExternalDeps(t *testing.T) {
|
|||||||
}
|
}
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
|
// TestCheckExternalDepNoBeadsDirectory verifies that CheckExternalDep
|
||||||
|
// correctly reports "no beads database" when the target project exists
|
||||||
|
// but has no .beads directory (bd-mv6h).
|
||||||
|
func TestCheckExternalDepNoBeadsDirectory(t *testing.T) {
|
||||||
|
ctx := context.Background()
|
||||||
|
|
||||||
|
// Create a project directory WITHOUT .beads
|
||||||
|
projectDir, err := os.MkdirTemp("", "beads-no-beads-test-*")
|
||||||
|
if err != nil {
|
||||||
|
t.Fatalf("failed to create temp dir: %v", err)
|
||||||
|
}
|
||||||
|
defer os.RemoveAll(projectDir)
|
||||||
|
|
||||||
|
// Initialize config if not already done
|
||||||
|
if err := config.Initialize(); err != nil {
|
||||||
|
t.Fatalf("failed to initialize config: %v", err)
|
||||||
|
}
|
||||||
|
|
||||||
|
// Configure external_projects to point to the directory
|
||||||
|
oldProjects := config.GetExternalProjects()
|
||||||
|
defer func() {
|
||||||
|
if oldProjects != nil {
|
||||||
|
config.Set("external_projects", oldProjects)
|
||||||
|
} else {
|
||||||
|
config.Set("external_projects", map[string]string{})
|
||||||
|
}
|
||||||
|
}()
|
||||||
|
|
||||||
|
config.Set("external_projects", map[string]string{
|
||||||
|
"no-beads-project": projectDir,
|
||||||
|
})
|
||||||
|
|
||||||
|
// Check the external dep - should report "no beads database"
|
||||||
|
status := CheckExternalDep(ctx, "external:no-beads-project:some-capability")
|
||||||
|
|
||||||
|
if status.Satisfied {
|
||||||
|
t.Error("Expected external dep to be unsatisfied when target has no .beads directory")
|
||||||
|
}
|
||||||
|
if status.Reason != "project has no beads database" {
|
||||||
|
t.Errorf("Expected reason 'project has no beads database', got: %s", status.Reason)
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
|
// TestCheckExternalDepInvalidFormats verifies that CheckExternalDep
|
||||||
|
// correctly handles various invalid external ref formats (bd-mv6h).
|
||||||
|
func TestCheckExternalDepInvalidFormats(t *testing.T) {
|
||||||
|
ctx := context.Background()
|
||||||
|
|
||||||
|
tests := []struct {
|
||||||
|
name string
|
||||||
|
ref string
|
||||||
|
wantReason string
|
||||||
|
}{
|
||||||
|
{
|
||||||
|
name: "not external prefix",
|
||||||
|
ref: "bd-xyz",
|
||||||
|
wantReason: "not an external reference",
|
||||||
|
},
|
||||||
|
{
|
||||||
|
name: "missing capability",
|
||||||
|
ref: "external:project",
|
||||||
|
wantReason: "invalid format (expected external:project:capability)",
|
||||||
|
},
|
||||||
|
{
|
||||||
|
name: "empty project",
|
||||||
|
ref: "external::capability",
|
||||||
|
wantReason: "missing project or capability",
|
||||||
|
},
|
||||||
|
{
|
||||||
|
name: "empty capability",
|
||||||
|
ref: "external:project:",
|
||||||
|
wantReason: "missing project or capability",
|
||||||
|
},
|
||||||
|
{
|
||||||
|
name: "only external prefix",
|
||||||
|
ref: "external:",
|
||||||
|
wantReason: "invalid format (expected external:project:capability)",
|
||||||
|
},
|
||||||
|
{
|
||||||
|
name: "unconfigured project",
|
||||||
|
ref: "external:unconfigured-project:capability",
|
||||||
|
wantReason: "project not configured in external_projects",
|
||||||
|
},
|
||||||
|
}
|
||||||
|
|
||||||
|
// Initialize config if not already done
|
||||||
|
if err := config.Initialize(); err != nil {
|
||||||
|
t.Fatalf("failed to initialize config: %v", err)
|
||||||
|
}
|
||||||
|
|
||||||
|
// Ensure no external projects are configured for some tests
|
||||||
|
oldProjects := config.GetExternalProjects()
|
||||||
|
defer func() {
|
||||||
|
if oldProjects != nil {
|
||||||
|
config.Set("external_projects", oldProjects)
|
||||||
|
}
|
||||||
|
}()
|
||||||
|
config.Set("external_projects", map[string]string{})
|
||||||
|
|
||||||
|
for _, tt := range tests {
|
||||||
|
t.Run(tt.name, func(t *testing.T) {
|
||||||
|
status := CheckExternalDep(ctx, tt.ref)
|
||||||
|
if status.Satisfied {
|
||||||
|
t.Errorf("Expected unsatisfied for %q", tt.ref)
|
||||||
|
}
|
||||||
|
if status.Reason != tt.wantReason {
|
||||||
|
t.Errorf("Expected reason %q, got %q", tt.wantReason, status.Reason)
|
||||||
|
}
|
||||||
|
})
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|||||||
Reference in New Issue
Block a user