cmd/bd: add unit coverage for show/update/close

Co-authored-by: factory-droid[bot] <138933559+factory-droid[bot]@users.noreply.github.com>

cmd/bd/cli_coverage_show_test.go

@@ -1,3 +1,5 @@
+//go:build e2e
+
 package main
 
 import (

cmd/bd/show.go

@@ -663,8 +663,8 @@ var updateCmd = &cobra.Command{
 				fmt.Fprintf(os.Stderr, "Error getting %s: %v\n", id, err)
 				continue
 			}
-			if issue != nil && issue.IsTemplate {
-				fmt.Fprintf(os.Stderr, "Error: cannot update template %s: templates are read-only; use 'bd molecule instantiate' to create a work item\n", id)
+			if err := validateIssueUpdatable(id, issue); err != nil {
+				fmt.Fprintf(os.Stderr, "%s\n", err)
 				continue
 			}
 
@@ -683,48 +683,21 @@ var updateCmd = &cobra.Command{
 			}
 
 			// Handle label operations
-			// Set labels (replaces all existing labels)
-			if setLabels, ok := updates["set_labels"].([]string); ok && len(setLabels) > 0 {
-				// Get current labels
-				currentLabels, err := store.GetLabels(ctx, id)
-				if err != nil {
-					fmt.Fprintf(os.Stderr, "Error getting labels for %s: %v\n", id, err)
+			var setLabels, addLabels, removeLabels []string
+			if v, ok := updates["set_labels"].([]string); ok {
+				setLabels = v
+			}
+			if v, ok := updates["add_labels"].([]string); ok {
+				addLabels = v
+			}
+			if v, ok := updates["remove_labels"].([]string); ok {
+				removeLabels = v
+			}
+			if len(setLabels) > 0 || len(addLabels) > 0 || len(removeLabels) > 0 {
+				if err := applyLabelUpdates(ctx, store, id, actor, setLabels, addLabels, removeLabels); err != nil {
+					fmt.Fprintf(os.Stderr, "Error updating labels for %s: %v\n", id, err)
 					continue
 				}
-				// Remove all current labels
-				for _, label := range currentLabels {
-					if err := store.RemoveLabel(ctx, id, label, actor); err != nil {
-						fmt.Fprintf(os.Stderr, "Error removing label %s from %s: %v\n", label, id, err)
-						continue
-					}
-				}
-				// Add new labels
-				for _, label := range setLabels {
-					if err := store.AddLabel(ctx, id, label, actor); err != nil {
-						fmt.Fprintf(os.Stderr, "Error setting label %s on %s: %v\n", label, id, err)
-						continue
-					}
-				}
-			}
-
-			// Add labels
-			if addLabels, ok := updates["add_labels"].([]string); ok {
-				for _, label := range addLabels {
-					if err := store.AddLabel(ctx, id, label, actor); err != nil {
-						fmt.Fprintf(os.Stderr, "Error adding label %s to %s: %v\n", label, id, err)
-						continue
-					}
-				}
-			}
-
-			// Remove labels
-			if removeLabels, ok := updates["remove_labels"].([]string); ok {
-				for _, label := range removeLabels {
-					if err := store.RemoveLabel(ctx, id, label, actor); err != nil {
-						fmt.Fprintf(os.Stderr, "Error removing label %s from %s: %v\n", label, id, err)
-						continue
-					}
-				}
 			}
 
 			// Run update hook (bd-kwro.8)
@@ -993,14 +966,8 @@ var closeCmd = &cobra.Command{
 				if showErr == nil {
 					var issue types.Issue
 					if json.Unmarshal(showResp.Data, &issue) == nil {
-						// Check if issue is a template (beads-1ra): templates are read-only
-						if issue.IsTemplate {
-							fmt.Fprintf(os.Stderr, "Error: cannot close template %s: templates are read-only\n", id)
-							continue
-						}
-						// Check if issue is pinned (bd-6v2)
-						if !force && issue.Status == types.StatusPinned {
-							fmt.Fprintf(os.Stderr, "Error: cannot close pinned issue %s (use --force to override)\n", id)
+						if err := validateIssueClosable(id, &issue, force); err != nil {
+							fmt.Fprintf(os.Stderr, "%s\n", err)
 							continue
 						}
 					}
@@ -1051,20 +1018,11 @@ var closeCmd = &cobra.Command{
 			// Get issue for checks
 			issue, _ := store.GetIssue(ctx, id)
 
-			// Check if issue is a template (beads-1ra): templates are read-only
-			if issue != nil && issue.IsTemplate {
-				fmt.Fprintf(os.Stderr, "Error: cannot close template %s: templates are read-only\n", id)
+			if err := validateIssueClosable(id, issue, force); err != nil {
+				fmt.Fprintf(os.Stderr, "%s\n", err)
 				continue
 			}
 
-			// Check if issue is pinned (bd-6v2)
-			if !force {
-				if issue != nil && issue.Status == types.StatusPinned {
-					fmt.Fprintf(os.Stderr, "Error: cannot close pinned issue %s (use --force to override)\n", id)
-					continue
-				}
-			}
-
 			if err := store.CloseIssue(ctx, id, reason, actor); err != nil {
 				fmt.Fprintf(os.Stderr, "Error closing %s: %v\n", id, err)
 				continue
@@ -1291,15 +1249,13 @@ func findRepliesTo(ctx context.Context, issueID string, daemonClient *rpc.Client
 		return ""
 	}
 	// Direct mode - query storage
-	if sqliteStore, ok := store.(*sqlite.SQLiteStorage); ok {
-		deps, err := sqliteStore.GetDependenciesWithMetadata(ctx, issueID)
-		if err != nil {
-			return ""
-		}
-		for _, dep := range deps {
-			if dep.DependencyType == types.DepRepliesTo {
-				return dep.ID
-			}
+	deps, err := store.GetDependencyRecords(ctx, issueID)
+	if err != nil {
+		return ""
+	}
+	for _, dep := range deps {
+		if dep.Type == types.DepRepliesTo {
+			return dep.DependsOnID
 		}
 	}
 	return ""
@@ -1348,7 +1304,25 @@ func findReplies(ctx context.Context, issueID string, daemonClient *rpc.Client,
 		}
 		return replies
 	}
-	return nil
+
+	allDeps, err := store.GetAllDependencyRecords(ctx)
+	if err != nil {
+		return nil
+	}
+
+	var replies []*types.Issue
+	for childID, deps := range allDeps {
+		for _, dep := range deps {
+			if dep.Type == types.DepRepliesTo && dep.DependsOnID == issueID {
+				issue, _ := store.GetIssue(ctx, childID)
+				if issue != nil {
+					replies = append(replies, issue)
+				}
+			}
+		}
+	}
+
+	return replies
 }
 
 func init() {

cmd/bd/show_unit_helpers.go

@@ -0,0 +1,68 @@
+package main
+
+import (
+	"context"
+	"fmt"
+
+	"github.com/steveyegge/beads/internal/storage"
+	"github.com/steveyegge/beads/internal/types"
+)
+
+func validateIssueUpdatable(id string, issue *types.Issue) error {
+	if issue == nil {
+		return nil
+	}
+	if issue.IsTemplate {
+		return fmt.Errorf("Error: cannot update template %s: templates are read-only; use 'bd molecule instantiate' to create a work item", id)
+	}
+	return nil
+}
+
+func validateIssueClosable(id string, issue *types.Issue, force bool) error {
+	if issue == nil {
+		return nil
+	}
+	if issue.IsTemplate {
+		return fmt.Errorf("Error: cannot close template %s: templates are read-only", id)
+	}
+	if !force && issue.Status == types.StatusPinned {
+		return fmt.Errorf("Error: cannot close pinned issue %s (use --force to override)", id)
+	}
+	return nil
+}
+
+func applyLabelUpdates(ctx context.Context, st storage.Storage, issueID, actor string, setLabels, addLabels, removeLabels []string) error {
+	// Set labels (replaces all existing labels)
+	if len(setLabels) > 0 {
+		currentLabels, err := st.GetLabels(ctx, issueID)
+		if err != nil {
+			return err
+		}
+		for _, label := range currentLabels {
+			if err := st.RemoveLabel(ctx, issueID, label, actor); err != nil {
+				return err
+			}
+		}
+		for _, label := range setLabels {
+			if err := st.AddLabel(ctx, issueID, label, actor); err != nil {
+				return err
+			}
+		}
+	}
+
+	// Add labels
+	for _, label := range addLabels {
+		if err := st.AddLabel(ctx, issueID, label, actor); err != nil {
+			return err
+		}
+	}
+
+	// Remove labels
+	for _, label := range removeLabels {
+		if err := st.RemoveLabel(ctx, issueID, label, actor); err != nil {
+			return err
+		}
+	}
+
+	return nil
+}

cmd/bd/show_unit_helpers_test.go

@@ -0,0 +1,139 @@
+package main
+
+import (
+	"context"
+	"testing"
+
+	"github.com/steveyegge/beads/internal/storage/memory"
+	"github.com/steveyegge/beads/internal/types"
+)
+
+func TestValidateIssueUpdatable(t *testing.T) {
+	if err := validateIssueUpdatable("x", nil); err != nil {
+		t.Fatalf("expected nil error, got %v", err)
+	}
+	if err := validateIssueUpdatable("x", &types.Issue{IsTemplate: false}); err != nil {
+		t.Fatalf("expected nil error, got %v", err)
+	}
+	if err := validateIssueUpdatable("bd-1", &types.Issue{IsTemplate: true}); err == nil {
+		t.Fatalf("expected error")
+	}
+}
+
+func TestValidateIssueClosable(t *testing.T) {
+	if err := validateIssueClosable("x", nil, false); err != nil {
+		t.Fatalf("expected nil error, got %v", err)
+	}
+	if err := validateIssueClosable("bd-1", &types.Issue{IsTemplate: true}, false); err == nil {
+		t.Fatalf("expected template close error")
+	}
+	if err := validateIssueClosable("bd-2", &types.Issue{Status: types.StatusPinned}, false); err == nil {
+		t.Fatalf("expected pinned close error")
+	}
+	if err := validateIssueClosable("bd-2", &types.Issue{Status: types.StatusPinned}, true); err != nil {
+		t.Fatalf("expected pinned close to succeed with force, got %v", err)
+	}
+}
+
+func TestApplyLabelUpdates_SetAddRemove(t *testing.T) {
+	ctx := context.Background()
+	st := memory.New("")
+	if err := st.SetConfig(ctx, "issue_prefix", "test"); err != nil {
+		t.Fatalf("SetConfig: %v", err)
+	}
+
+	issue := &types.Issue{Title: "x", Status: types.StatusOpen, Priority: 2, IssueType: types.TypeTask}
+	if err := st.CreateIssue(ctx, issue, "tester"); err != nil {
+		t.Fatalf("CreateIssue: %v", err)
+	}
+
+	_ = st.AddLabel(ctx, issue.ID, "old1", "tester")
+	_ = st.AddLabel(ctx, issue.ID, "old2", "tester")
+
+	if err := applyLabelUpdates(ctx, st, issue.ID, "tester", []string{"a", "b"}, []string{"b", "c"}, []string{"a"}); err != nil {
+		t.Fatalf("applyLabelUpdates: %v", err)
+	}
+	labels, _ := st.GetLabels(ctx, issue.ID)
+	if len(labels) != 2 {
+		t.Fatalf("expected 2 labels, got %v", labels)
+	}
+	// Order is not guaranteed.
+	foundB := false
+	foundC := false
+	for _, l := range labels {
+		if l == "b" {
+			foundB = true
+		}
+		if l == "c" {
+			foundC = true
+		}
+		if l == "old1" || l == "old2" || l == "a" {
+			t.Fatalf("unexpected label %q in %v", l, labels)
+		}
+	}
+	if !foundB || !foundC {
+		t.Fatalf("expected labels b and c, got %v", labels)
+	}
+}
+
+func TestApplyLabelUpdates_AddRemoveOnly(t *testing.T) {
+	ctx := context.Background()
+	st := memory.New("")
+	issue := &types.Issue{Title: "x", Status: types.StatusOpen, Priority: 2, IssueType: types.TypeTask}
+	if err := st.CreateIssue(ctx, issue, "tester"); err != nil {
+		t.Fatalf("CreateIssue: %v", err)
+	}
+
+	_ = st.AddLabel(ctx, issue.ID, "a", "tester")
+	if err := applyLabelUpdates(ctx, st, issue.ID, "tester", nil, []string{"b"}, []string{"a"}); err != nil {
+		t.Fatalf("applyLabelUpdates: %v", err)
+	}
+	labels, _ := st.GetLabels(ctx, issue.ID)
+	if len(labels) != 1 || labels[0] != "b" {
+		t.Fatalf("expected [b], got %v", labels)
+	}
+}
+
+func TestFindRepliesToAndReplies_WorksWithMemoryStorage(t *testing.T) {
+	ctx := context.Background()
+	st := memory.New("")
+	if err := st.SetConfig(ctx, "issue_prefix", "test"); err != nil {
+		t.Fatalf("SetConfig: %v", err)
+	}
+
+	root := &types.Issue{Title: "root", Status: types.StatusOpen, Priority: 2, IssueType: types.TypeMessage, Sender: "a", Assignee: "b"}
+	reply1 := &types.Issue{Title: "r1", Status: types.StatusOpen, Priority: 2, IssueType: types.TypeMessage, Sender: "b", Assignee: "a"}
+	reply2 := &types.Issue{Title: "r2", Status: types.StatusOpen, Priority: 2, IssueType: types.TypeMessage, Sender: "a", Assignee: "b"}
+	if err := st.CreateIssue(ctx, root, "tester"); err != nil {
+		t.Fatalf("CreateIssue(root): %v", err)
+	}
+	if err := st.CreateIssue(ctx, reply1, "tester"); err != nil {
+		t.Fatalf("CreateIssue(reply1): %v", err)
+	}
+	if err := st.CreateIssue(ctx, reply2, "tester"); err != nil {
+		t.Fatalf("CreateIssue(reply2): %v", err)
+	}
+
+	if err := st.AddDependency(ctx, &types.Dependency{IssueID: reply1.ID, DependsOnID: root.ID, Type: types.DepRepliesTo}, "tester"); err != nil {
+		t.Fatalf("AddDependency(reply1->root): %v", err)
+	}
+	if err := st.AddDependency(ctx, &types.Dependency{IssueID: reply2.ID, DependsOnID: reply1.ID, Type: types.DepRepliesTo}, "tester"); err != nil {
+		t.Fatalf("AddDependency(reply2->reply1): %v", err)
+	}
+
+	if got := findRepliesTo(ctx, root.ID, nil, st); got != "" {
+		t.Fatalf("expected root replies-to to be empty, got %q", got)
+	}
+	if got := findRepliesTo(ctx, reply2.ID, nil, st); got != reply1.ID {
+		t.Fatalf("expected reply2 parent %q, got %q", reply1.ID, got)
+	}
+
+	rootReplies := findReplies(ctx, root.ID, nil, st)
+	if len(rootReplies) != 1 || rootReplies[0].ID != reply1.ID {
+		t.Fatalf("expected root replies [%s], got %+v", reply1.ID, rootReplies)
+	}
+	r1Replies := findReplies(ctx, reply1.ID, nil, st)
+	if len(r1Replies) != 1 || r1Replies[0].ID != reply2.ID {
+		t.Fatalf("expected reply1 replies [%s], got %+v", reply2.ID, r1Replies)
+	}
+}