Centralize validation patterns with composable validators (bd-jbqx)

Add composable issue validators to internal/validation package:
- IssueValidator type with Chain() composition function
- Exists(), NotTemplate(), NotPinned(), NotClosed(), NotHooked() validators
- HasStatus(), HasType() for checking allowed values
- ForUpdate(), ForClose(), ForDelete(), ForReopen() convenience chains

Update cmd/bd/show_unit_helpers.go to use centralized validators instead
of duplicated inline validation logic. This enables consistent validation
across all commands with a single source of truth.

🤖 Generated with [Claude Code](https://claude.com/claude-code)

Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>

cmd/bd/show_unit_helpers.go

@@ -2,33 +2,29 @@ package main
 
 import (
 	"context"
-	"fmt"
 
 	"github.com/steveyegge/beads/internal/storage"
 	"github.com/steveyegge/beads/internal/types"
+	"github.com/steveyegge/beads/internal/validation"
 )
 
+// validateIssueUpdatable checks if an issue can be updated.
+// Uses the centralized validation package for consistency.
 func validateIssueUpdatable(id string, issue *types.Issue) error {
-	if issue == nil {
+	// Note: We use NotTemplate() directly instead of ForUpdate() to maintain
-		return nil
+	// backward compatibility - the original didn't check for nil issues.
-	}
+	return validation.NotTemplate()(id, issue)
-	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
 }
 
+// validateIssueClosable checks if an issue can be closed.
+// Uses the centralized validation package for consistency.
 func validateIssueClosable(id string, issue *types.Issue, force bool) error {
-	if issue == nil {
+	// Note: We use individual validators instead of ForClose() to maintain
-		return nil
+	// backward compatibility - the original didn't check for nil issues.
-	}
+	return validation.Chain(
-	if issue.IsTemplate {
+		validation.NotTemplate(),
-		return fmt.Errorf("Error: cannot close template %s: templates are read-only", id)
+		validation.NotPinned(force),
-	}
+	)(id, issue)
-	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 {

internal/validation/issue.go

@@ -0,0 +1,156 @@
+package validation
+
+import (
+	"fmt"
+
+	"github.com/steveyegge/beads/internal/types"
+)
+
+// IssueValidator validates an issue and returns an error if validation fails.
+// Validators can be composed using Chain() for complex validation logic.
+type IssueValidator func(id string, issue *types.Issue) error
+
+// Chain composes multiple validators into a single validator.
+// Validators are executed in order and the first error stops the chain.
+func Chain(validators ...IssueValidator) IssueValidator {
+	return func(id string, issue *types.Issue) error {
+		for _, v := range validators {
+			if err := v(id, issue); err != nil {
+				return err
+			}
+		}
+		return nil
+	}
+}
+
+// Exists validates that an issue is not nil.
+func Exists() IssueValidator {
+	return func(id string, issue *types.Issue) error {
+		if issue == nil {
+			return fmt.Errorf("issue %s not found", id)
+		}
+		return nil
+	}
+}
+
+// NotTemplate validates that an issue is not a template.
+// Templates are read-only and cannot be modified.
+func NotTemplate() IssueValidator {
+	return func(id string, issue *types.Issue) error {
+		if issue == nil {
+			return nil // Let Exists() handle nil check if needed
+		}
+		if issue.IsTemplate {
+			return fmt.Errorf("cannot modify template %s: templates are read-only; use 'bd mol pour' to create a work item", id)
+		}
+		return nil
+	}
+}
+
+// NotPinned validates that an issue is not pinned.
+// Returns an error if the issue is pinned, unless force is true.
+func NotPinned(force bool) IssueValidator {
+	return func(id string, issue *types.Issue) error {
+		if issue == nil {
+			return nil // Let Exists() handle nil check if needed
+		}
+		if !force && issue.Status == types.StatusPinned {
+			return fmt.Errorf("cannot modify pinned issue %s (use --force to override)", id)
+		}
+		return nil
+	}
+}
+
+// NotClosed validates that an issue is not already closed.
+func NotClosed() IssueValidator {
+	return func(id string, issue *types.Issue) error {
+		if issue == nil {
+			return nil
+		}
+		if issue.Status == types.StatusClosed {
+			return fmt.Errorf("issue %s is already closed", id)
+		}
+		return nil
+	}
+}
+
+// NotHooked validates that an issue is not in hooked status.
+func NotHooked(force bool) IssueValidator {
+	return func(id string, issue *types.Issue) error {
+		if issue == nil {
+			return nil
+		}
+		if !force && issue.Status == types.StatusHooked {
+			return fmt.Errorf("cannot modify hooked issue %s (use --force to override)", id)
+		}
+		return nil
+	}
+}
+
+// HasStatus validates that an issue has one of the allowed statuses.
+func HasStatus(allowed ...types.Status) IssueValidator {
+	return func(id string, issue *types.Issue) error {
+		if issue == nil {
+			return nil
+		}
+		for _, status := range allowed {
+			if issue.Status == status {
+				return nil
+			}
+		}
+		return fmt.Errorf("issue %s has status %s, expected one of: %v", id, issue.Status, allowed)
+	}
+}
+
+// HasType validates that an issue has one of the allowed types.
+func HasType(allowed ...types.IssueType) IssueValidator {
+	return func(id string, issue *types.Issue) error {
+		if issue == nil {
+			return nil
+		}
+		for _, t := range allowed {
+			if issue.IssueType == t {
+				return nil
+			}
+		}
+		return fmt.Errorf("issue %s has type %s, expected one of: %v", id, issue.IssueType, allowed)
+	}
+}
+
+// ForUpdate returns a validator chain for update operations.
+// Validates: issue exists and is not a template.
+func ForUpdate() IssueValidator {
+	return Chain(
+		Exists(),
+		NotTemplate(),
+	)
+}
+
+// ForClose returns a validator chain for close operations.
+// Validates: issue exists, is not a template, and is not pinned (unless force).
+func ForClose(force bool) IssueValidator {
+	return Chain(
+		Exists(),
+		NotTemplate(),
+		NotPinned(force),
+	)
+}
+
+// ForDelete returns a validator chain for delete operations.
+// Validates: issue exists and is not a template.
+func ForDelete() IssueValidator {
+	return Chain(
+		Exists(),
+		NotTemplate(),
+	)
+}
+
+// ForReopen returns a validator chain for reopen operations.
+// Validates: issue exists, is not a template, and is closed.
+func ForReopen() IssueValidator {
+	return Chain(
+		Exists(),
+		NotTemplate(),
+		HasStatus(types.StatusClosed),
+	)
+}

internal/validation/issue_test.go

@@ -0,0 +1,367 @@
+package validation
+
+import (
+	"testing"
+
+	"github.com/steveyegge/beads/internal/types"
+)
+
+func TestExists(t *testing.T) {
+	tests := []struct {
+		name    string
+		issue   *types.Issue
+		wantErr bool
+	}{
+		{
+			name:    "nil issue returns error",
+			issue:   nil,
+			wantErr: true,
+		},
+		{
+			name:    "non-nil issue passes",
+			issue:   &types.Issue{ID: "bd-test"},
+			wantErr: false,
+		},
+	}
+
+	for _, tt := range tests {
+		t.Run(tt.name, func(t *testing.T) {
+			err := Exists()("bd-test", tt.issue)
+			if (err != nil) != tt.wantErr {
+				t.Errorf("Exists() error = %v, wantErr %v", err, tt.wantErr)
+			}
+		})
+	}
+}
+
+func TestNotTemplate(t *testing.T) {
+	tests := []struct {
+		name    string
+		issue   *types.Issue
+		wantErr bool
+	}{
+		{
+			name:    "nil issue passes (delegated check)",
+			issue:   nil,
+			wantErr: false,
+		},
+		{
+			name:    "non-template passes",
+			issue:   &types.Issue{ID: "bd-test", IsTemplate: false},
+			wantErr: false,
+		},
+		{
+			name:    "template returns error",
+			issue:   &types.Issue{ID: "bd-test", IsTemplate: true},
+			wantErr: true,
+		},
+	}
+
+	for _, tt := range tests {
+		t.Run(tt.name, func(t *testing.T) {
+			err := NotTemplate()("bd-test", tt.issue)
+			if (err != nil) != tt.wantErr {
+				t.Errorf("NotTemplate() error = %v, wantErr %v", err, tt.wantErr)
+			}
+		})
+	}
+}
+
+func TestNotPinned(t *testing.T) {
+	tests := []struct {
+		name    string
+		issue   *types.Issue
+		force   bool
+		wantErr bool
+	}{
+		{
+			name:    "nil issue passes",
+			issue:   nil,
+			force:   false,
+			wantErr: false,
+		},
+		{
+			name:    "open issue passes",
+			issue:   &types.Issue{ID: "bd-test", Status: types.StatusOpen},
+			force:   false,
+			wantErr: false,
+		},
+		{
+			name:    "pinned issue without force fails",
+			issue:   &types.Issue{ID: "bd-test", Status: types.StatusPinned},
+			force:   false,
+			wantErr: true,
+		},
+		{
+			name:    "pinned issue with force passes",
+			issue:   &types.Issue{ID: "bd-test", Status: types.StatusPinned},
+			force:   true,
+			wantErr: false,
+		},
+	}
+
+	for _, tt := range tests {
+		t.Run(tt.name, func(t *testing.T) {
+			err := NotPinned(tt.force)("bd-test", tt.issue)
+			if (err != nil) != tt.wantErr {
+				t.Errorf("NotPinned() error = %v, wantErr %v", err, tt.wantErr)
+			}
+		})
+	}
+}
+
+func TestNotClosed(t *testing.T) {
+	tests := []struct {
+		name    string
+		issue   *types.Issue
+		wantErr bool
+	}{
+		{
+			name:    "nil issue passes",
+			issue:   nil,
+			wantErr: false,
+		},
+		{
+			name:    "open issue passes",
+			issue:   &types.Issue{ID: "bd-test", Status: types.StatusOpen},
+			wantErr: false,
+		},
+		{
+			name:    "closed issue fails",
+			issue:   &types.Issue{ID: "bd-test", Status: types.StatusClosed},
+			wantErr: true,
+		},
+	}
+
+	for _, tt := range tests {
+		t.Run(tt.name, func(t *testing.T) {
+			err := NotClosed()("bd-test", tt.issue)
+			if (err != nil) != tt.wantErr {
+				t.Errorf("NotClosed() error = %v, wantErr %v", err, tt.wantErr)
+			}
+		})
+	}
+}
+
+func TestChain(t *testing.T) {
+	tests := []struct {
+		name       string
+		issue      *types.Issue
+		validators []IssueValidator
+		wantErr    bool
+	}{
+		{
+			name:       "empty chain passes",
+			issue:      &types.Issue{ID: "bd-test"},
+			validators: []IssueValidator{},
+			wantErr:    false,
+		},
+		{
+			name:  "all validators pass",
+			issue: &types.Issue{ID: "bd-test", Status: types.StatusOpen},
+			validators: []IssueValidator{
+				Exists(),
+				NotTemplate(),
+				NotPinned(false),
+			},
+			wantErr: false,
+		},
+		{
+			name:  "first validator fails stops chain",
+			issue: nil,
+			validators: []IssueValidator{
+				Exists(),
+				NotTemplate(),
+			},
+			wantErr: true,
+		},
+		{
+			name:  "middle validator fails",
+			issue: &types.Issue{ID: "bd-test", IsTemplate: true},
+			validators: []IssueValidator{
+				Exists(),
+				NotTemplate(),
+				NotPinned(false),
+			},
+			wantErr: true,
+		},
+	}
+
+	for _, tt := range tests {
+		t.Run(tt.name, func(t *testing.T) {
+			chain := Chain(tt.validators...)
+			err := chain("bd-test", tt.issue)
+			if (err != nil) != tt.wantErr {
+				t.Errorf("Chain() error = %v, wantErr %v", err, tt.wantErr)
+			}
+		})
+	}
+}
+
+func TestHasStatus(t *testing.T) {
+	tests := []struct {
+		name    string
+		issue   *types.Issue
+		allowed []types.Status
+		wantErr bool
+	}{
+		{
+			name:    "nil issue passes",
+			issue:   nil,
+			allowed: []types.Status{types.StatusOpen},
+			wantErr: false,
+		},
+		{
+			name:    "matching status passes",
+			issue:   &types.Issue{ID: "bd-test", Status: types.StatusOpen},
+			allowed: []types.Status{types.StatusOpen},
+			wantErr: false,
+		},
+		{
+			name:    "one of multiple allowed passes",
+			issue:   &types.Issue{ID: "bd-test", Status: types.StatusClosed},
+			allowed: []types.Status{types.StatusOpen, types.StatusClosed},
+			wantErr: false,
+		},
+		{
+			name:    "non-matching status fails",
+			issue:   &types.Issue{ID: "bd-test", Status: types.StatusPinned},
+			allowed: []types.Status{types.StatusOpen, types.StatusClosed},
+			wantErr: true,
+		},
+	}
+
+	for _, tt := range tests {
+		t.Run(tt.name, func(t *testing.T) {
+			err := HasStatus(tt.allowed...)("bd-test", tt.issue)
+			if (err != nil) != tt.wantErr {
+				t.Errorf("HasStatus() error = %v, wantErr %v", err, tt.wantErr)
+			}
+		})
+	}
+}
+
+func TestHasType(t *testing.T) {
+	tests := []struct {
+		name    string
+		issue   *types.Issue
+		allowed []types.IssueType
+		wantErr bool
+	}{
+		{
+			name:    "nil issue passes",
+			issue:   nil,
+			allowed: []types.IssueType{types.TypeTask},
+			wantErr: false,
+		},
+		{
+			name:    "matching type passes",
+			issue:   &types.Issue{ID: "bd-test", IssueType: types.TypeTask},
+			allowed: []types.IssueType{types.TypeTask},
+			wantErr: false,
+		},
+		{
+			name:    "one of multiple allowed passes",
+			issue:   &types.Issue{ID: "bd-test", IssueType: types.TypeBug},
+			allowed: []types.IssueType{types.TypeTask, types.TypeBug},
+			wantErr: false,
+		},
+		{
+			name:    "non-matching type fails",
+			issue:   &types.Issue{ID: "bd-test", IssueType: types.TypeEpic},
+			allowed: []types.IssueType{types.TypeTask, types.TypeBug},
+			wantErr: true,
+		},
+	}
+
+	for _, tt := range tests {
+		t.Run(tt.name, func(t *testing.T) {
+			err := HasType(tt.allowed...)("bd-test", tt.issue)
+			if (err != nil) != tt.wantErr {
+				t.Errorf("HasType() error = %v, wantErr %v", err, tt.wantErr)
+			}
+		})
+	}
+}
+
+func TestForUpdate(t *testing.T) {
+	tests := []struct {
+		name    string
+		issue   *types.Issue
+		wantErr bool
+	}{
+		{
+			name:    "nil issue fails",
+			issue:   nil,
+			wantErr: true,
+		},
+		{
+			name:    "template fails",
+			issue:   &types.Issue{ID: "bd-test", IsTemplate: true},
+			wantErr: true,
+		},
+		{
+			name:    "regular issue passes",
+			issue:   &types.Issue{ID: "bd-test", IsTemplate: false},
+			wantErr: false,
+		},
+	}
+
+	for _, tt := range tests {
+		t.Run(tt.name, func(t *testing.T) {
+			err := ForUpdate()("bd-test", tt.issue)
+			if (err != nil) != tt.wantErr {
+				t.Errorf("ForUpdate() error = %v, wantErr %v", err, tt.wantErr)
+			}
+		})
+	}
+}
+
+func TestForClose(t *testing.T) {
+	tests := []struct {
+		name    string
+		issue   *types.Issue
+		force   bool
+		wantErr bool
+	}{
+		{
+			name:    "nil issue fails",
+			issue:   nil,
+			force:   false,
+			wantErr: true,
+		},
+		{
+			name:    "template fails",
+			issue:   &types.Issue{ID: "bd-test", IsTemplate: true},
+			force:   false,
+			wantErr: true,
+		},
+		{
+			name:    "pinned without force fails",
+			issue:   &types.Issue{ID: "bd-test", Status: types.StatusPinned},
+			force:   false,
+			wantErr: true,
+		},
+		{
+			name:    "pinned with force passes",
+			issue:   &types.Issue{ID: "bd-test", Status: types.StatusPinned},
+			force:   true,
+			wantErr: false,
+		},
+		{
+			name:    "regular open issue passes",
+			issue:   &types.Issue{ID: "bd-test", Status: types.StatusOpen},
+			force:   false,
+			wantErr: false,
+		},
+	}
+
+	for _, tt := range tests {
+		t.Run(tt.name, func(t *testing.T) {
+			err := ForClose(tt.force)("bd-test", tt.issue)
+			if (err != nil) != tt.wantErr {
+				t.Errorf("ForClose() error = %v, wantErr %v", err, tt.wantErr)
+			}
+		})
+	}
+}