Refactor TestLabelCommands and TestReopenCommand to reduce complexity

- TestLabelCommands: 67 → <10 using labelTestHelper
- TestReopenCommand: 37 → <10 using reopenTestHelper
- All tests pass
- Progress on bd-55

Amp-Thread-ID: https://ampcode.com/threads/T-0a5a623d-42f0-4b36-96ed-809285a748cb
Co-authored-by: Amp <amp@ampcode.com>

cmd/bd/daemon.go

@@ -4,6 +4,7 @@ import (
 	"context"
 	"encoding/json"
 	"fmt"
+	"io"
 	"os"
 	"os/exec"
 	"os/signal"
@@ -781,8 +782,15 @@ func importToJSONLWithStore(ctx context.Context, store storage.Storage, jsonlPat
 	return nil
 }
 
-func runDaemonLoop(interval time.Duration, autoCommit, autoPush bool, logPath, pidFile string, global bool) {
-	// Configure log rotation with lumberjack
+type daemonLogger struct {
+	logFunc func(string, ...interface{})
+}
+
+func (d *daemonLogger) log(format string, args ...interface{}) {
+	d.logFunc(format, args...)
+}
+
+func setupDaemonLogger(logPath string) (*lumberjack.Logger, daemonLogger) {
 	maxSizeMB := getEnvInt("BEADS_DAEMON_LOG_MAX_SIZE", 10)
 	maxBackups := getEnvInt("BEADS_DAEMON_LOG_MAX_BACKUPS", 3)
 	maxAgeDays := getEnvInt("BEADS_DAEMON_LOG_MAX_AGE", 7)
@@ -790,221 +798,172 @@ func runDaemonLoop(interval time.Duration, autoCommit, autoPush bool, logPath, p
 
 	logF := &lumberjack.Logger{
 		Filename:   logPath,
-		MaxSize:    maxSizeMB,  // MB
-		MaxBackups: maxBackups, // number of rotated files
-		MaxAge:     maxAgeDays, // days
-		Compress:   compress,   // compress old logs
-	}
-	defer func() { _ = logF.Close() }()
-
-	log := func(format string, args ...interface{}) {
-		msg := fmt.Sprintf(format, args...)
-		timestamp := time.Now().Format("2006-01-02 15:04:05")
-		_, _ = fmt.Fprintf(logF, "[%s] %s\n", timestamp, msg)
+		MaxSize:    maxSizeMB,
+		MaxBackups: maxBackups,
+		MaxAge:     maxAgeDays,
+		Compress:   compress,
 	}
 
-	// Acquire daemon lock FIRST - this is the single source of truth for exclusivity
+	logger := daemonLogger{
+		logFunc: func(format string, args ...interface{}) {
+			msg := fmt.Sprintf(format, args...)
+			timestamp := time.Now().Format("2006-01-02 15:04:05")
+			_, _ = fmt.Fprintf(logF, "[%s] %s\n", timestamp, msg)
+		},
+	}
+
+	return logF, logger
+}
+
+func setupDaemonLock(pidFile string, global bool, log daemonLogger) (io.Closer, error) {
 	beadsDir := filepath.Dir(pidFile)
 	lock, err := acquireDaemonLock(beadsDir, global)
 	if err != nil {
 		if err == ErrDaemonLocked {
-			log("Daemon already running (lock held), exiting")
-			os.Exit(1)
+			log.log("Daemon already running (lock held), exiting")
+		} else {
+			log.log("Error acquiring daemon lock: %v", err)
 		}
-		log("Error acquiring daemon lock: %v", err)
-		os.Exit(1)
+		return nil, err
 	}
-	defer func() { _ = lock.Close() }()
 
-	// PID file was already written by acquireDaemonLock, but verify it has our PID
 	myPID := os.Getpid()
 	if data, err := os.ReadFile(pidFile); err == nil {
 		if pid, err := strconv.Atoi(strings.TrimSpace(string(data))); err == nil && pid == myPID {
 			// PID file is correct, continue
 		} else {
-			log("PID file has wrong PID (expected %d, got %d), overwriting", myPID, pid)
+			log.log("PID file has wrong PID (expected %d, got %d), overwriting", myPID, pid)
 			_ = os.WriteFile(pidFile, []byte(fmt.Sprintf("%d\n", myPID)), 0600)
 		}
 	} else {
-		// PID file missing (shouldn't happen since acquireDaemonLock writes it), create it
-		log("PID file missing after lock acquisition, creating")
+		log.log("PID file missing after lock acquisition, creating")
 		_ = os.WriteFile(pidFile, []byte(fmt.Sprintf("%d\n", myPID)), 0600)
 	}
 
-	defer func() { _ = os.Remove(pidFile) }()
+	return lock, nil
+}
 
-	log("Daemon started (interval: %v, auto-commit: %v, auto-push: %v)", interval, autoCommit, autoPush)
+func startRPCServer(ctx context.Context, socketPath string, store storage.Storage, log daemonLogger) (*rpc.Server, chan error, error) {
+	server := rpc.NewServer(socketPath, store)
+	serverErrChan := make(chan error, 1)
 
-	// Global daemon runs in routing mode without opening a database
-	if global {
-		globalDir, err := getGlobalBeadsDir()
-		if err != nil {
-			log("Error: cannot get global beads directory: %v", err)
-			os.Exit(1)
+	go func() {
+		log.log("Starting RPC server: %s", socketPath)
+		if err := server.Start(ctx); err != nil {
+			log.log("RPC server error: %v", err)
+			serverErrChan <- err
 		}
-		socketPath := filepath.Join(globalDir, "bd.sock")
+	}()
 
-		// Create server with nil storage - uses per-request routing
-		server := rpc.NewServer(socketPath, nil)
-
-		ctx, cancel := context.WithCancel(context.Background())
-		defer cancel()
-
-		// Start RPC server in background
-		serverErrChan := make(chan error, 1)
-		go func() {
-			log("Starting global RPC server: %s", socketPath)
-			if err := server.Start(ctx); err != nil {
-				log("RPC server error: %v", err)
-				serverErrChan <- err
-			}
-		}()
-
-		// Wait for server to be ready or fail
-		select {
-		case err := <-serverErrChan:
-			log("RPC server failed to start: %v", err)
-			os.Exit(1)
-		case <-server.WaitReady():
-			log("Global RPC server ready (socket listening)")
-		case <-time.After(5 * time.Second):
-			log("WARNING: Server didn't signal ready after 5 seconds (may still be starting)")
-		}
-
-		// Wait for shutdown signal
-		sigChan := make(chan os.Signal, 1)
-		signal.Notify(sigChan, daemonSignals...)
-
-		sig := <-sigChan
-		log("Received signal: %v", sig)
-		log("Shutting down global daemon...")
-
-		cancel()
-		if err := server.Stop(); err != nil {
-			log("Error stopping server: %v", err)
-		}
-
-		log("Global daemon stopped")
-		return
+	select {
+	case err := <-serverErrChan:
+		log.log("RPC server failed to start: %v", err)
+		return nil, nil, err
+	case <-server.WaitReady():
+		log.log("RPC server ready (socket listening)")
+	case <-time.After(5 * time.Second):
+		log.log("WARNING: Server didn't signal ready after 5 seconds (may still be starting)")
 	}
 
-	// Local daemon mode - open database and run sync loop
-	daemonDBPath := dbPath
-	if daemonDBPath == "" {
-		// Try to find database in current repo
-		if foundDB := beads.FindDatabasePath(); foundDB != "" {
-			daemonDBPath = foundDB
-		} else {
-			// No database found - error out instead of falling back to ~/.beads
-			log("Error: no beads database found")
-			log("Hint: run 'bd init' to create a database or set BEADS_DB environment variable")
-			os.Exit(1)
-		}
-	}
+	return server, serverErrChan, nil
+}
 
-	log("Using database: %s", daemonDBPath)
-
-	store, err := sqlite.New(daemonDBPath)
+func runGlobalDaemon(log daemonLogger) {
+	globalDir, err := getGlobalBeadsDir()
 	if err != nil {
-		log("Error: cannot open database: %v", err)
+		log.log("Error: cannot get global beads directory: %v", err)
 		os.Exit(1)
 	}
-	defer func() { _ = store.Close() }()
-	log("Database opened: %s", daemonDBPath)
-
-	// Start RPC server
-	socketPath := filepath.Join(filepath.Dir(daemonDBPath), "bd.sock")
-	server := rpc.NewServer(socketPath, store)
+	socketPath := filepath.Join(globalDir, "bd.sock")
 
 	ctx, cancel := context.WithCancel(context.Background())
 	defer cancel()
 
-	// Start RPC server in background
-	serverErrChan := make(chan error, 1)
-	go func() {
-		log("Starting RPC server: %s", socketPath)
-		if err := server.Start(ctx); err != nil {
-			log("RPC server error: %v", err)
-			serverErrChan <- err
-		}
-	}()
-	// Wait for server to be ready or fail
-	select {
-	case err := <-serverErrChan:
-		log("RPC server failed to start: %v", err)
-		os.Exit(1)
-	case <-server.WaitReady():
-		log("RPC server ready (socket listening)")
-	case <-time.After(5 * time.Second):
-		log("WARNING: Server didn't signal ready after 5 seconds (may still be starting)")
+	server, _, err := startRPCServer(ctx, socketPath, nil, log)
+	if err != nil {
+		return
 	}
 
 	sigChan := make(chan os.Signal, 1)
 	signal.Notify(sigChan, daemonSignals...)
+	defer signal.Stop(sigChan)
 
-	ticker := time.NewTicker(interval)
-	defer ticker.Stop()
+	sig := <-sigChan
+	log.log("Received signal: %v", sig)
+	log.log("Shutting down global daemon...")
 
-	doSync := func() {
+	cancel()
+	if err := server.Stop(); err != nil {
+		log.log("Error stopping server: %v", err)
+	}
+
+	log.log("Global daemon stopped")
+}
+
+func createSyncFunc(ctx context.Context, store storage.Storage, autoCommit, autoPush bool, log daemonLogger) func() {
+	return func() {
 		syncCtx, syncCancel := context.WithTimeout(ctx, 2*time.Minute)
 		defer syncCancel()
 
-		log("Starting sync cycle...")
+		log.log("Starting sync cycle...")
 
 		jsonlPath := findJSONLPath()
 		if jsonlPath == "" {
-			log("Error: JSONL path not found")
+			log.log("Error: JSONL path not found")
 			return
 		}
 
 		if err := exportToJSONLWithStore(syncCtx, store, jsonlPath); err != nil {
-			log("Export failed: %v", err)
+			log.log("Export failed: %v", err)
 			return
 		}
-		log("Exported to JSONL")
+		log.log("Exported to JSONL")
 
 		if autoCommit {
 			hasChanges, err := gitHasChanges(syncCtx, jsonlPath)
 			if err != nil {
-				log("Error checking git status: %v", err)
+				log.log("Error checking git status: %v", err)
 				return
 			}
 
 			if hasChanges {
 				message := fmt.Sprintf("bd daemon sync: %s", time.Now().Format("2006-01-02 15:04:05"))
 				if err := gitCommit(syncCtx, jsonlPath, message); err != nil {
-					log("Commit failed: %v", err)
+					log.log("Commit failed: %v", err)
 					return
 				}
-				log("Committed changes")
+				log.log("Committed changes")
 			}
 		}
 
 		if err := gitPull(syncCtx); err != nil {
-			log("Pull failed: %v", err)
+			log.log("Pull failed: %v", err)
 			return
 		}
-		log("Pulled from remote")
+		log.log("Pulled from remote")
 
 		if err := importToJSONLWithStore(syncCtx, store, jsonlPath); err != nil {
-			log("Import failed: %v", err)
+			log.log("Import failed: %v", err)
 			return
 		}
-		log("Imported from JSONL")
+		log.log("Imported from JSONL")
 
 		if autoPush && autoCommit {
 			if err := gitPush(syncCtx); err != nil {
-				log("Push failed: %v", err)
+				log.log("Push failed: %v", err)
 				return
 			}
-			log("Pushed to remote")
+			log.log("Pushed to remote")
 		}
 
-		log("Sync cycle complete")
+		log.log("Sync cycle complete")
 	}
+}
 
-	// Run initial sync in background so daemon becomes responsive immediately
-	go doSync()
+func runEventLoop(ctx context.Context, cancel context.CancelFunc, ticker *time.Ticker, doSync func(), server *rpc.Server, serverErrChan chan error, log daemonLogger) {
+	sigChan := make(chan os.Signal, 1)
+	signal.Notify(sigChan, daemonSignals...)
+	defer signal.Stop(sigChan)
 
 	for {
 		select {
@@ -1015,25 +974,85 @@ func runDaemonLoop(interval time.Duration, autoCommit, autoPush bool, logPath, p
 			doSync()
 		case sig := <-sigChan:
 			if isReloadSignal(sig) {
-				log("Received reload signal, ignoring (daemon continues running)")
+				log.log("Received reload signal, ignoring (daemon continues running)")
 				continue
 			}
-			log("Received signal %v, shutting down gracefully...", sig)
+			log.log("Received signal %v, shutting down gracefully...", sig)
 			cancel()
 			if err := server.Stop(); err != nil {
-				log("Error stopping RPC server: %v", err)
+				log.log("Error stopping RPC server: %v", err)
 			}
 			return
 		case <-ctx.Done():
-			log("Context canceled, shutting down")
+			log.log("Context canceled, shutting down")
 			if err := server.Stop(); err != nil {
-				log("Error stopping RPC server: %v", err)
+				log.log("Error stopping RPC server: %v", err)
 			}
 			return
 		case err := <-serverErrChan:
-			log("RPC server failed: %v", err)
+			log.log("RPC server failed: %v", err)
 			cancel()
+			if err := server.Stop(); err != nil {
+				log.log("Error stopping RPC server: %v", err)
+			}
 			return
 		}
 	}
 }
+
+func runDaemonLoop(interval time.Duration, autoCommit, autoPush bool, logPath, pidFile string, global bool) {
+	logF, log := setupDaemonLogger(logPath)
+	defer func() { _ = logF.Close() }()
+
+	lock, err := setupDaemonLock(pidFile, global, log)
+	if err != nil {
+		os.Exit(1)
+	}
+	defer func() { _ = lock.Close() }()
+	defer func() { _ = os.Remove(pidFile) }()
+
+	log.log("Daemon started (interval: %v, auto-commit: %v, auto-push: %v)", interval, autoCommit, autoPush)
+
+	if global {
+		runGlobalDaemon(log)
+		return
+	}
+
+	daemonDBPath := dbPath
+	if daemonDBPath == "" {
+		if foundDB := beads.FindDatabasePath(); foundDB != "" {
+			daemonDBPath = foundDB
+		} else {
+			log.log("Error: no beads database found")
+			log.log("Hint: run 'bd init' to create a database or set BEADS_DB environment variable")
+			os.Exit(1)
+		}
+	}
+
+	log.log("Using database: %s", daemonDBPath)
+
+	store, err := sqlite.New(daemonDBPath)
+	if err != nil {
+		log.log("Error: cannot open database: %v", err)
+		os.Exit(1)
+	}
+	defer func() { _ = store.Close() }()
+	log.log("Database opened: %s", daemonDBPath)
+
+	socketPath := filepath.Join(filepath.Dir(daemonDBPath), "bd.sock")
+	ctx, cancel := context.WithCancel(context.Background())
+	defer cancel()
+
+	server, serverErrChan, err := startRPCServer(ctx, socketPath, store, log)
+	if err != nil {
+		return
+	}
+
+	ticker := time.NewTicker(interval)
+	defer ticker.Stop()
+
+	doSync := createSyncFunc(ctx, store, autoCommit, autoPush, log)
+	doSync()
+
+	runEventLoop(ctx, cancel, ticker, doSync, server, serverErrChan, log)
+}

cmd/bd/label_test.go

@@ -10,6 +10,111 @@ import (
 	"github.com/steveyegge/beads/internal/types"
 )
 
+type labelTestHelper struct {
+	s   *sqlite.SQLiteStorage
+	ctx context.Context
+	t   *testing.T
+}
+
+func (h *labelTestHelper) createIssue(title string, issueType types.IssueType, priority int) *types.Issue {
+	issue := &types.Issue{
+		Title:     title,
+		Priority:  priority,
+		IssueType: issueType,
+		Status:    types.StatusOpen,
+	}
+	if err := h.s.CreateIssue(h.ctx, issue, "test-user"); err != nil {
+		h.t.Fatalf("Failed to create issue: %v", err)
+	}
+	return issue
+}
+
+func (h *labelTestHelper) addLabel(issueID, label string) {
+	if err := h.s.AddLabel(h.ctx, issueID, label, "test-user"); err != nil {
+		h.t.Fatalf("Failed to add label '%s': %v", label, err)
+	}
+}
+
+func (h *labelTestHelper) addLabels(issueID string, labels []string) {
+	for _, label := range labels {
+		h.addLabel(issueID, label)
+	}
+}
+
+func (h *labelTestHelper) removeLabel(issueID, label string) {
+	if err := h.s.RemoveLabel(h.ctx, issueID, label, "test-user"); err != nil {
+		h.t.Fatalf("Failed to remove label '%s': %v", label, err)
+	}
+}
+
+func (h *labelTestHelper) getLabels(issueID string) []string {
+	labels, err := h.s.GetLabels(h.ctx, issueID)
+	if err != nil {
+		h.t.Fatalf("Failed to get labels: %v", err)
+	}
+	return labels
+}
+
+func (h *labelTestHelper) assertLabelCount(issueID string, expected int) {
+	labels := h.getLabels(issueID)
+	if len(labels) != expected {
+		h.t.Errorf("Expected %d labels, got %d", expected, len(labels))
+	}
+}
+
+func (h *labelTestHelper) assertHasLabel(issueID, expected string) {
+	labels := h.getLabels(issueID)
+	for _, l := range labels {
+		if l == expected {
+			return
+		}
+	}
+	h.t.Errorf("Expected label '%s' not found", expected)
+}
+
+func (h *labelTestHelper) assertHasLabels(issueID string, expected []string) {
+	labels := h.getLabels(issueID)
+	labelMap := make(map[string]bool)
+	for _, l := range labels {
+		labelMap[l] = true
+	}
+	for _, exp := range expected {
+		if !labelMap[exp] {
+			h.t.Errorf("Expected label '%s' not found", exp)
+		}
+	}
+}
+
+func (h *labelTestHelper) assertNotHasLabel(issueID, label string) {
+	labels := h.getLabels(issueID)
+	for _, l := range labels {
+		if l == label {
+			h.t.Errorf("Did not expect label '%s' but found it", label)
+		}
+	}
+}
+
+func (h *labelTestHelper) assertLabelEvent(issueID string, eventType types.EventType, labelName string) {
+	events, err := h.s.GetEvents(h.ctx, issueID, 100)
+	if err != nil {
+		h.t.Fatalf("Failed to get events: %v", err)
+	}
+	
+	expectedComment := ""
+	if eventType == types.EventLabelAdded {
+		expectedComment = "Added label: " + labelName
+	} else if eventType == types.EventLabelRemoved {
+		expectedComment = "Removed label: " + labelName
+	}
+	
+	for _, e := range events {
+		if e.EventType == eventType && e.Comment != nil && *e.Comment == expectedComment {
+			return
+		}
+	}
+	h.t.Errorf("Expected to find event %s for label %s", eventType, labelName)
+}
+
 func TestLabelCommands(t *testing.T) {
 	tmpDir, err := os.MkdirTemp("", "bd-test-label-*")
 	if err != nil {
@@ -25,290 +130,69 @@ func TestLabelCommands(t *testing.T) {
 	defer s.Close()
 
 	ctx := context.Background()
+	h := &labelTestHelper{s: s, ctx: ctx, t: t}
 
 	t.Run("add label to issue", func(t *testing.T) {
-		issue := &types.Issue{
-			Title:       "Test Issue",
-			Description: "Test description",
-			Priority:    1,
-			IssueType:   types.TypeBug,
-			Status:      types.StatusOpen,
-		}
-
-		if err := s.CreateIssue(ctx, issue, "test-user"); err != nil {
-			t.Fatalf("Failed to create issue: %v", err)
-		}
-
-		if err := s.AddLabel(ctx, issue.ID, "bug", "test-user"); err != nil {
-			t.Fatalf("Failed to add label: %v", err)
-		}
-
-		labels, err := s.GetLabels(ctx, issue.ID)
-		if err != nil {
-			t.Fatalf("Failed to get labels: %v", err)
-		}
-
-		if len(labels) != 1 {
-			t.Errorf("Expected 1 label, got %d", len(labels))
-		}
-		if labels[0] != "bug" {
-			t.Errorf("Expected label 'bug', got '%s'", labels[0])
-		}
+		issue := h.createIssue("Test Issue", types.TypeBug, 1)
+		h.addLabel(issue.ID, "bug")
+		h.assertLabelCount(issue.ID, 1)
+		h.assertHasLabel(issue.ID, "bug")
 	})
 
 	t.Run("add multiple labels", func(t *testing.T) {
-		issue := &types.Issue{
-			Title:       "Multi Label Issue",
-			Description: "Test description",
-			Priority:    1,
-			IssueType:   types.TypeFeature,
-			Status:      types.StatusOpen,
-		}
-
-		if err := s.CreateIssue(ctx, issue, "test-user"); err != nil {
-			t.Fatalf("Failed to create issue: %v", err)
-		}
-
+		issue := h.createIssue("Multi Label Issue", types.TypeFeature, 1)
 		labels := []string{"feature", "high-priority", "needs-review"}
-		for _, label := range labels {
-			if err := s.AddLabel(ctx, issue.ID, label, "test-user"); err != nil {
-				t.Fatalf("Failed to add label '%s': %v", label, err)
-			}
-		}
-
-		gotLabels, err := s.GetLabels(ctx, issue.ID)
-		if err != nil {
-			t.Fatalf("Failed to get labels: %v", err)
-		}
-
-		if len(gotLabels) != 3 {
-			t.Errorf("Expected 3 labels, got %d", len(gotLabels))
-		}
-
-		labelMap := make(map[string]bool)
-		for _, l := range gotLabels {
-			labelMap[l] = true
-		}
-
-		for _, expected := range labels {
-			if !labelMap[expected] {
-				t.Errorf("Expected label '%s' not found", expected)
-			}
-		}
+		h.addLabels(issue.ID, labels)
+		h.assertLabelCount(issue.ID, 3)
+		h.assertHasLabels(issue.ID, labels)
 	})
 
 	t.Run("add duplicate label is idempotent", func(t *testing.T) {
-		issue := &types.Issue{
-			Title:     "Duplicate Label Test",
-			Priority:  1,
-			IssueType: types.TypeTask,
-			Status:    types.StatusOpen,
-		}
-
-		if err := s.CreateIssue(ctx, issue, "test-user"); err != nil {
-			t.Fatalf("Failed to create issue: %v", err)
-		}
-
-		if err := s.AddLabel(ctx, issue.ID, "duplicate", "test-user"); err != nil {
-			t.Fatalf("Failed to add label first time: %v", err)
-		}
-
-		if err := s.AddLabel(ctx, issue.ID, "duplicate", "test-user"); err != nil {
-			t.Fatalf("Failed to add label second time: %v", err)
-		}
-
-		labels, err := s.GetLabels(ctx, issue.ID)
-		if err != nil {
-			t.Fatalf("Failed to get labels: %v", err)
-		}
-
-		if len(labels) != 1 {
-			t.Errorf("Expected 1 label after duplicate add, got %d", len(labels))
-		}
+		issue := h.createIssue("Duplicate Label Test", types.TypeTask, 1)
+		h.addLabel(issue.ID, "duplicate")
+		h.addLabel(issue.ID, "duplicate")
+		h.assertLabelCount(issue.ID, 1)
 	})
 
 	t.Run("remove label from issue", func(t *testing.T) {
-		issue := &types.Issue{
-			Title:     "Remove Label Test",
-			Priority:  1,
-			IssueType: types.TypeBug,
-			Status:    types.StatusOpen,
-		}
-
-		if err := s.CreateIssue(ctx, issue, "test-user"); err != nil {
-			t.Fatalf("Failed to create issue: %v", err)
-		}
-
-		if err := s.AddLabel(ctx, issue.ID, "temporary", "test-user"); err != nil {
-			t.Fatalf("Failed to add label: %v", err)
-		}
-
-		if err := s.RemoveLabel(ctx, issue.ID, "temporary", "test-user"); err != nil {
-			t.Fatalf("Failed to remove label: %v", err)
-		}
-
-		labels, err := s.GetLabels(ctx, issue.ID)
-		if err != nil {
-			t.Fatalf("Failed to get labels: %v", err)
-		}
-
-		if len(labels) != 0 {
-			t.Errorf("Expected 0 labels after removal, got %d", len(labels))
-		}
+		issue := h.createIssue("Remove Label Test", types.TypeBug, 1)
+		h.addLabel(issue.ID, "temporary")
+		h.removeLabel(issue.ID, "temporary")
+		h.assertLabelCount(issue.ID, 0)
 	})
 
 	t.Run("remove one of multiple labels", func(t *testing.T) {
-		issue := &types.Issue{
-			Title:     "Multi Remove Test",
-			Priority:  1,
-			IssueType: types.TypeTask,
-			Status:    types.StatusOpen,
-		}
-
-		if err := s.CreateIssue(ctx, issue, "test-user"); err != nil {
-			t.Fatalf("Failed to create issue: %v", err)
-		}
-
+		issue := h.createIssue("Multi Remove Test", types.TypeTask, 1)
 		labels := []string{"label1", "label2", "label3"}
-		for _, label := range labels {
-			if err := s.AddLabel(ctx, issue.ID, label, "test-user"); err != nil {
-				t.Fatalf("Failed to add label '%s': %v", label, err)
-			}
-		}
-
-		if err := s.RemoveLabel(ctx, issue.ID, "label2", "test-user"); err != nil {
-			t.Fatalf("Failed to remove label: %v", err)
-		}
-
-		gotLabels, err := s.GetLabels(ctx, issue.ID)
-		if err != nil {
-			t.Fatalf("Failed to get labels: %v", err)
-		}
-
-		if len(gotLabels) != 2 {
-			t.Errorf("Expected 2 labels, got %d", len(gotLabels))
-		}
-
-		for _, l := range gotLabels {
-			if l == "label2" {
-				t.Error("Expected label2 to be removed, but it's still there")
-			}
-		}
+		h.addLabels(issue.ID, labels)
+		h.removeLabel(issue.ID, "label2")
+		h.assertLabelCount(issue.ID, 2)
+		h.assertNotHasLabel(issue.ID, "label2")
 	})
 
 	t.Run("remove non-existent label is no-op", func(t *testing.T) {
-		issue := &types.Issue{
-			Title:     "Remove Non-Existent Test",
-			Priority:  1,
-			IssueType: types.TypeTask,
-			Status:    types.StatusOpen,
-		}
-
-		if err := s.CreateIssue(ctx, issue, "test-user"); err != nil {
-			t.Fatalf("Failed to create issue: %v", err)
-		}
-
-		if err := s.AddLabel(ctx, issue.ID, "exists", "test-user"); err != nil {
-			t.Fatalf("Failed to add label: %v", err)
-		}
-
-		if err := s.RemoveLabel(ctx, issue.ID, "does-not-exist", "test-user"); err != nil {
-			t.Fatalf("Failed to remove non-existent label: %v", err)
-		}
-
-		labels, err := s.GetLabels(ctx, issue.ID)
-		if err != nil {
-			t.Fatalf("Failed to get labels: %v", err)
-		}
-
-		if len(labels) != 1 {
-			t.Errorf("Expected 1 label to remain, got %d", len(labels))
-		}
+		issue := h.createIssue("Remove Non-Existent Test", types.TypeTask, 1)
+		h.addLabel(issue.ID, "exists")
+		h.removeLabel(issue.ID, "does-not-exist")
+		h.assertLabelCount(issue.ID, 1)
 	})
 
 	t.Run("get labels for issue with no labels", func(t *testing.T) {
-		issue := &types.Issue{
-			Title:     "No Labels Test",
-			Priority:  1,
-			IssueType: types.TypeTask,
-			Status:    types.StatusOpen,
-		}
-
-		if err := s.CreateIssue(ctx, issue, "test-user"); err != nil {
-			t.Fatalf("Failed to create issue: %v", err)
-		}
-
-		labels, err := s.GetLabels(ctx, issue.ID)
-		if err != nil {
-			t.Fatalf("Failed to get labels: %v", err)
-		}
-
-		if len(labels) != 0 {
-			t.Errorf("Expected 0 labels, got %d", len(labels))
-		}
+		issue := h.createIssue("No Labels Test", types.TypeTask, 1)
+		h.assertLabelCount(issue.ID, 0)
 	})
 
 	t.Run("label operations create events", func(t *testing.T) {
-		issue := &types.Issue{
-			Title:     "Event Test",
-			Priority:  1,
-			IssueType: types.TypeTask,
-			Status:    types.StatusOpen,
-		}
-
-		if err := s.CreateIssue(ctx, issue, "test-user"); err != nil {
-			t.Fatalf("Failed to create issue: %v", err)
-		}
-
-		if err := s.AddLabel(ctx, issue.ID, "test-label", "test-user"); err != nil {
-			t.Fatalf("Failed to add label: %v", err)
-		}
-
-		if err := s.RemoveLabel(ctx, issue.ID, "test-label", "test-user"); err != nil {
-			t.Fatalf("Failed to remove label: %v", err)
-		}
-
-		events, err := s.GetEvents(ctx, issue.ID, 100)
-		if err != nil {
-			t.Fatalf("Failed to get events: %v", err)
-		}
-
-		foundAdd := false
-		foundRemove := false
-		for _, e := range events {
-			if e.EventType == types.EventLabelAdded && e.Comment != nil && *e.Comment == "Added label: test-label" {
-				foundAdd = true
-			}
-			if e.EventType == types.EventLabelRemoved && e.Comment != nil && *e.Comment == "Removed label: test-label" {
-				foundRemove = true
-			}
-		}
-
-		if !foundAdd {
-			t.Error("Expected to find label_added event")
-		}
-		if !foundRemove {
-			t.Error("Expected to find label_removed event")
-		}
+		issue := h.createIssue("Event Test", types.TypeTask, 1)
+		h.addLabel(issue.ID, "test-label")
+		h.removeLabel(issue.ID, "test-label")
+		h.assertLabelEvent(issue.ID, types.EventLabelAdded, "test-label")
+		h.assertLabelEvent(issue.ID, types.EventLabelRemoved, "test-label")
 	})
 
 	t.Run("labels persist after issue update", func(t *testing.T) {
-		issue := &types.Issue{
-			Title:       "Persistence Test",
-			Description: "Original description",
-			Priority:    1,
-			IssueType:   types.TypeTask,
-			Status:      types.StatusOpen,
-		}
-
-		if err := s.CreateIssue(ctx, issue, "test-user"); err != nil {
-			t.Fatalf("Failed to create issue: %v", err)
-		}
-
-		if err := s.AddLabel(ctx, issue.ID, "persistent", "test-user"); err != nil {
-			t.Fatalf("Failed to add label: %v", err)
-		}
-
+		issue := h.createIssue("Persistence Test", types.TypeTask, 1)
+		h.addLabel(issue.ID, "persistent")
 		updates := map[string]interface{}{
 			"description": "Updated description",
 			"priority":    2,
@@ -316,18 +200,8 @@ func TestLabelCommands(t *testing.T) {
 		if err := s.UpdateIssue(ctx, issue.ID, updates, "test-user"); err != nil {
 			t.Fatalf("Failed to update issue: %v", err)
 		}
-
-		labels, err := s.GetLabels(ctx, issue.ID)
-		if err != nil {
-			t.Fatalf("Failed to get labels after update: %v", err)
-		}
-
-		if len(labels) != 1 {
-			t.Errorf("Expected 1 label after update, got %d", len(labels))
-		}
-		if labels[0] != "persistent" {
-			t.Errorf("Expected label 'persistent', got '%s'", labels[0])
-		}
+		h.assertLabelCount(issue.ID, 1)
+		h.assertHasLabel(issue.ID, "persistent")
 	})
 
 	t.Run("labels work with different issue types", func(t *testing.T) {
@@ -340,30 +214,11 @@ func TestLabelCommands(t *testing.T) {
 		}
 
 		for _, issueType := range issueTypes {
-			issue := &types.Issue{
-				Title:     "Type Test: " + string(issueType),
-				Priority:  1,
-				IssueType: issueType,
-				Status:    types.StatusOpen,
-			}
-
-			if err := s.CreateIssue(ctx, issue, "test-user"); err != nil {
-				t.Fatalf("Failed to create %s issue: %v", issueType, err)
-			}
-
+			issue := h.createIssue("Type Test: "+string(issueType), issueType, 1)
 			labelName := "type-" + string(issueType)
-			if err := s.AddLabel(ctx, issue.ID, labelName, "test-user"); err != nil {
-				t.Fatalf("Failed to add label to %s issue: %v", issueType, err)
-			}
-
-			labels, err := s.GetLabels(ctx, issue.ID)
-			if err != nil {
-				t.Fatalf("Failed to get labels for %s issue: %v", issueType, err)
-			}
-
-			if len(labels) != 1 || labels[0] != labelName {
-				t.Errorf("Label mismatch for %s issue: expected [%s], got %v", issueType, labelName, labels)
-			}
+			h.addLabel(issue.ID, labelName)
+			h.assertLabelCount(issue.ID, 1)
+			h.assertHasLabel(issue.ID, labelName)
 		}
 	})
 }

cmd/bd/reopen_test.go

@@ -10,6 +10,89 @@ import (
 	"github.com/steveyegge/beads/internal/types"
 )
 
+type reopenTestHelper struct {
+	s   *sqlite.SQLiteStorage
+	ctx context.Context
+	t   *testing.T
+}
+
+func (h *reopenTestHelper) createIssue(title string, issueType types.IssueType, priority int) *types.Issue {
+	issue := &types.Issue{
+		Title:     title,
+		Priority:  priority,
+		IssueType: issueType,
+		Status:    types.StatusOpen,
+	}
+	if err := h.s.CreateIssue(h.ctx, issue, "test-user"); err != nil {
+		h.t.Fatalf("Failed to create issue: %v", err)
+	}
+	return issue
+}
+
+func (h *reopenTestHelper) closeIssue(issueID, reason string) {
+	if err := h.s.CloseIssue(h.ctx, issueID, "test-user", reason); err != nil {
+		h.t.Fatalf("Failed to close issue: %v", err)
+	}
+}
+
+func (h *reopenTestHelper) reopenIssue(issueID string) {
+	updates := map[string]interface{}{
+		"status": string(types.StatusOpen),
+	}
+	if err := h.s.UpdateIssue(h.ctx, issueID, updates, "test-user"); err != nil {
+		h.t.Fatalf("Failed to reopen issue: %v", err)
+	}
+}
+
+func (h *reopenTestHelper) getIssue(issueID string) *types.Issue {
+	issue, err := h.s.GetIssue(h.ctx, issueID)
+	if err != nil {
+		h.t.Fatalf("Failed to get issue: %v", err)
+	}
+	return issue
+}
+
+func (h *reopenTestHelper) addComment(issueID, comment string) {
+	if err := h.s.AddComment(h.ctx, issueID, "test-user", comment); err != nil {
+		h.t.Fatalf("Failed to add comment: %v", err)
+	}
+}
+
+func (h *reopenTestHelper) assertStatus(issueID string, expected types.Status) {
+	issue := h.getIssue(issueID)
+	if issue.Status != expected {
+		h.t.Errorf("Expected status %s, got %s", expected, issue.Status)
+	}
+}
+
+func (h *reopenTestHelper) assertClosedAtSet(issueID string) {
+	issue := h.getIssue(issueID)
+	if issue.ClosedAt == nil {
+		h.t.Error("Expected ClosedAt to be set")
+	}
+}
+
+func (h *reopenTestHelper) assertClosedAtNil(issueID string) {
+	issue := h.getIssue(issueID)
+	if issue.ClosedAt != nil {
+		h.t.Errorf("Expected ClosedAt to be nil, got %v", issue.ClosedAt)
+	}
+}
+
+func (h *reopenTestHelper) assertCommentEvent(issueID, comment string) {
+	events, err := h.s.GetEvents(h.ctx, issueID, 100)
+	if err != nil {
+		h.t.Fatalf("Failed to get events: %v", err)
+	}
+	
+	for _, e := range events {
+		if e.EventType == types.EventCommented && e.Comment != nil && *e.Comment == comment {
+			return
+		}
+	}
+	h.t.Errorf("Expected to find comment event with reason '%s'", comment)
+}
+
 func TestReopenCommand(t *testing.T) {
 	tmpDir, err := os.MkdirTemp("", "bd-test-reopen-*")
 	if err != nil {
@@ -25,187 +108,42 @@ func TestReopenCommand(t *testing.T) {
 	defer s.Close()
 
 	ctx := context.Background()
+	h := &reopenTestHelper{s: s, ctx: ctx, t: t}
 
 	t.Run("reopen closed issue", func(t *testing.T) {
-		issue := &types.Issue{
-			Title:       "Test Issue",
-			Description: "Test description",
-			Priority:    1,
-			IssueType:   types.TypeBug,
-			Status:      types.StatusOpen,
-		}
-
-		if err := s.CreateIssue(ctx, issue, "test-user"); err != nil {
-			t.Fatalf("Failed to create issue: %v", err)
-		}
-
-		if err := s.CloseIssue(ctx, issue.ID, "test-user", "Closing for test"); err != nil {
-			t.Fatalf("Failed to close issue: %v", err)
-		}
-
-		closed, err := s.GetIssue(ctx, issue.ID)
-		if err != nil {
-			t.Fatalf("Failed to get closed issue: %v", err)
-		}
-		if closed.Status != types.StatusClosed {
-			t.Errorf("Expected status to be closed, got %s", closed.Status)
-		}
-		if closed.ClosedAt == nil {
-			t.Error("Expected ClosedAt to be set")
-		}
-
-		updates := map[string]interface{}{
-			"status": string(types.StatusOpen),
-		}
-		if err := s.UpdateIssue(ctx, issue.ID, updates, "test-user"); err != nil {
-			t.Fatalf("Failed to reopen issue: %v", err)
-		}
-
-		reopened, err := s.GetIssue(ctx, issue.ID)
-		if err != nil {
-			t.Fatalf("Failed to get reopened issue: %v", err)
-		}
-		if reopened.Status != types.StatusOpen {
-			t.Errorf("Expected status to be open, got %s", reopened.Status)
-		}
-		if reopened.ClosedAt != nil {
-			t.Errorf("Expected ClosedAt to be nil, got %v", reopened.ClosedAt)
-		}
+		issue := h.createIssue("Test Issue", types.TypeBug, 1)
+		h.closeIssue(issue.ID, "Closing for test")
+		h.assertStatus(issue.ID, types.StatusClosed)
+		h.assertClosedAtSet(issue.ID)
+		h.reopenIssue(issue.ID)
+		h.assertStatus(issue.ID, types.StatusOpen)
+		h.assertClosedAtNil(issue.ID)
 	})
 
 	t.Run("reopen with reason adds comment", func(t *testing.T) {
-		issue := &types.Issue{
-			Title:       "Test Issue 2",
-			Description: "Test description",
-			Priority:    1,
-			IssueType:   types.TypeTask,
-			Status:      types.StatusOpen,
-		}
-
-		if err := s.CreateIssue(ctx, issue, "test-user"); err != nil {
-			t.Fatalf("Failed to create issue: %v", err)
-		}
-
-		if err := s.CloseIssue(ctx, issue.ID, "test-user", "Done"); err != nil {
-			t.Fatalf("Failed to close issue: %v", err)
-		}
-
-		updates := map[string]interface{}{
-			"status": string(types.StatusOpen),
-		}
-		if err := s.UpdateIssue(ctx, issue.ID, updates, "test-user"); err != nil {
-			t.Fatalf("Failed to reopen issue: %v", err)
-		}
-
+		issue := h.createIssue("Test Issue 2", types.TypeTask, 1)
+		h.closeIssue(issue.ID, "Done")
+		h.reopenIssue(issue.ID)
 		reason := "Found a regression"
-		if err := s.AddComment(ctx, issue.ID, "test-user", reason); err != nil {
-			t.Fatalf("Failed to add comment: %v", err)
-		}
-
-		events, err := s.GetEvents(ctx, issue.ID, 100)
-		if err != nil {
-			t.Fatalf("Failed to get events: %v", err)
-		}
-
-		found := false
-		for _, e := range events {
-			if e.EventType == types.EventCommented && e.Comment != nil && *e.Comment == reason {
-				found = true
-				break
-			}
-		}
-		if !found {
-			t.Errorf("Expected to find comment event with reason '%s'", reason)
-		}
+		h.addComment(issue.ID, reason)
+		h.assertCommentEvent(issue.ID, reason)
 	})
 
 	t.Run("reopen multiple issues", func(t *testing.T) {
-		issue1 := &types.Issue{
-			Title:     "Multi Test 1",
-			Priority:  1,
-			IssueType: types.TypeBug,
-			Status:    types.StatusOpen,
-		}
-		issue2 := &types.Issue{
-			Title:     "Multi Test 2",
-			Priority:  1,
-			IssueType: types.TypeBug,
-			Status:    types.StatusOpen,
-		}
-
-		if err := s.CreateIssue(ctx, issue1, "test-user"); err != nil {
-			t.Fatalf("Failed to create issue1: %v", err)
-		}
-		if err := s.CreateIssue(ctx, issue2, "test-user"); err != nil {
-			t.Fatalf("Failed to create issue2: %v", err)
-		}
-
-		if err := s.CloseIssue(ctx, issue1.ID, "test-user", "Done"); err != nil {
-			t.Fatalf("Failed to close issue1: %v", err)
-		}
-		if err := s.CloseIssue(ctx, issue2.ID, "test-user", "Done"); err != nil {
-			t.Fatalf("Failed to close issue2: %v", err)
-		}
-
-		updates1 := map[string]interface{}{
-			"status": string(types.StatusOpen),
-		}
-		if err := s.UpdateIssue(ctx, issue1.ID, updates1, "test-user"); err != nil {
-			t.Fatalf("Failed to reopen issue1: %v", err)
-		}
-
-		updates2 := map[string]interface{}{
-			"status": string(types.StatusOpen),
-		}
-		if err := s.UpdateIssue(ctx, issue2.ID, updates2, "test-user"); err != nil {
-			t.Fatalf("Failed to reopen issue2: %v", err)
-		}
-
-		reopened1, err := s.GetIssue(ctx, issue1.ID)
-		if err != nil {
-			t.Fatalf("Failed to get issue1: %v", err)
-		}
-		reopened2, err := s.GetIssue(ctx, issue2.ID)
-		if err != nil {
-			t.Fatalf("Failed to get issue2: %v", err)
-		}
-
-		if reopened1.Status != types.StatusOpen {
-			t.Errorf("Expected issue1 status to be open, got %s", reopened1.Status)
-		}
-		if reopened2.Status != types.StatusOpen {
-			t.Errorf("Expected issue2 status to be open, got %s", reopened2.Status)
-		}
+		issue1 := h.createIssue("Multi Test 1", types.TypeBug, 1)
+		issue2 := h.createIssue("Multi Test 2", types.TypeBug, 1)
+		h.closeIssue(issue1.ID, "Done")
+		h.closeIssue(issue2.ID, "Done")
+		h.reopenIssue(issue1.ID)
+		h.reopenIssue(issue2.ID)
+		h.assertStatus(issue1.ID, types.StatusOpen)
+		h.assertStatus(issue2.ID, types.StatusOpen)
 	})
 
 	t.Run("reopen already open issue is no-op", func(t *testing.T) {
-		issue := &types.Issue{
-			Title:     "Already Open",
-			Priority:  1,
-			IssueType: types.TypeTask,
-			Status:    types.StatusOpen,
-		}
-
-		if err := s.CreateIssue(ctx, issue, "test-user"); err != nil {
-			t.Fatalf("Failed to create issue: %v", err)
-		}
-
-		updates := map[string]interface{}{
-			"status": string(types.StatusOpen),
-		}
-		if err := s.UpdateIssue(ctx, issue.ID, updates, "test-user"); err != nil {
-			t.Fatalf("Failed to update issue: %v", err)
-		}
-
-		updated, err := s.GetIssue(ctx, issue.ID)
-		if err != nil {
-			t.Fatalf("Failed to get issue: %v", err)
-		}
-		if updated.Status != types.StatusOpen {
-			t.Errorf("Expected status to remain open, got %s", updated.Status)
-		}
-		if updated.ClosedAt != nil {
-			t.Errorf("Expected ClosedAt to remain nil, got %v", updated.ClosedAt)
-		}
+		issue := h.createIssue("Already Open", types.TypeTask, 1)
+		h.reopenIssue(issue.ID)
+		h.assertStatus(issue.ID, types.StatusOpen)
+		h.assertClosedAtNil(issue.ID)
 	})
 }