mutex everything

Author: tstromberg

cmd/goose/main.go

@@ -91,6 +91,7 @@ type App struct {
 	updateInterval      time.Duration
 	consecutiveFailures int
 	mu                  sync.RWMutex
+	menuMutex           sync.Mutex // Mutex to prevent concurrent menu rebuilds
 	enableAutoBrowser   bool
 	hideStaleIncoming   bool
 	hasPerformedInitialDiscovery bool // Track if we've done the first poll to distinguish from real state changes

cmd/goose/main_test.go

@@ -4,6 +4,7 @@ import (
 	"context"
 	"fmt"
 	"os"
+	"runtime"
 	"sync"
 	"testing"
 	"time"
@@ -196,8 +197,15 @@ func TestTrayIconRestoredAfterNetworkRecovery(t *testing.T) {
 	}
 	app.setTrayTitle()
 	initialTitle := mock.title
-	if initialTitle != "🪿 1" {
-		t.Errorf("Expected initial tray title '🪿 1', got %q", initialTitle)
+
+	// Expected title varies by platform
+	expectedTitle := "1/1" // Linux format
+	if runtime.GOOS == "darwin" {
+		expectedTitle = "1" // macOS format
+	}
+
+	if initialTitle != expectedTitle {
+		t.Errorf("Expected initial tray title %q, got %q", expectedTitle, initialTitle)
 	}
 
 	// Simulate network failure - updatePRs would set warning icon and return early
@@ -213,8 +221,8 @@ func TestTrayIconRestoredAfterNetworkRecovery(t *testing.T) {
 	// With our fix, setTrayTitle() is now called after successful fetch
 	app.setTrayTitle()
 	recoveredTitle := mock.title
-	if recoveredTitle != "🪿 1" {
-		t.Errorf("Expected tray title to be restored to '🪿 1' after recovery, got %q", recoveredTitle)
+	if recoveredTitle != expectedTitle {
+		t.Errorf("Expected tray title to be restored to %q after recovery, got %q", expectedTitle, recoveredTitle)
 	}
 }
 
@@ -447,15 +455,19 @@ func TestSoundPlaybackDuringTransitions(t *testing.T) {
 			// Actual sound playback is verified through integration testing.
 
 			// Set initial state
+			app.mu.Lock()
 			app.incoming = tt.initialIncoming
 			app.outgoing = tt.initialOutgoing
+			app.mu.Unlock()
 
 			// Run first check to establish baseline
 			app.checkForNewlyBlockedPRs(ctx)
 
 			// Update to new state
+			app.mu.Lock()
 			app.incoming = tt.updatedIncoming
 			app.outgoing = tt.updatedOutgoing
+			app.mu.Unlock()
 
 			// Run check again to detect transitions
 			app.checkForNewlyBlockedPRs(ctx)
@@ -465,6 +477,7 @@ func TestSoundPlaybackDuringTransitions(t *testing.T) {
 			if len(tt.expectedSounds) > 0 {
 				// Check that blocked PRs are tracked in previousBlockedPRs
 				blocked := 0
+				app.mu.RLock()
 				for _, pr := range app.incoming {
 					if pr.NeedsReview && app.previousBlockedPRs[pr.URL] {
 						blocked++
@@ -475,6 +488,7 @@ func TestSoundPlaybackDuringTransitions(t *testing.T) {
 						blocked++
 					}
 				}
+				app.mu.RUnlock()
 				if blocked == 0 {
 					t.Errorf("%s: expected blocked PRs to be tracked in previousBlockedPRs", tt.description)
 				}

cmd/goose/notifications.go

@@ -90,14 +90,10 @@ func (app *App) processNotifications(ctx context.Context) {
 			}
 		}
 
-		// Update menu if we sent notifications
-		if len(toNotify) > 0 {
-			slog.Debug("[NOTIFY] Updating menu after notifications")
-			app.updateMenu(ctx)
-		}
 	}()
 
-	// Update menu if we sent notifications
+	// Update menu immediately after sending notifications
+	// This needs to happen in the main thread to show the party popper emoji
 	if len(toNotify) > 0 {
 		slog.Info("[FLOW] Updating menu after sending notifications", "notified_count", len(toNotify))
 		app.updateMenu(ctx)

cmd/goose/systray_interface.go

@@ -2,6 +2,7 @@ package main
 
 import (
 	"log/slog"
+	"sync"
 
 	"github.com/energye/systray"
 )
@@ -54,15 +55,20 @@ func (*RealSystray) Quit() {
 
 // MockSystray implements SystrayInterface for testing.
 type MockSystray struct {
+	mu        sync.Mutex
 	title     string
 	menuItems []string
 }
 
 func (m *MockSystray) ResetMenu() {
+	m.mu.Lock()
+	defer m.mu.Unlock()
 	m.menuItems = nil
 }
 
 func (m *MockSystray) AddMenuItem(title, tooltip string) MenuItem {
+	m.mu.Lock()
+	defer m.mu.Unlock()
 	m.menuItems = append(m.menuItems, title)
 	// Return a MockMenuItem that won't panic when methods are called
 	return &MockMenuItem{
@@ -72,10 +78,14 @@ func (m *MockSystray) AddMenuItem(title, tooltip string) MenuItem {
 }
 
 func (m *MockSystray) AddSeparator() {
+	m.mu.Lock()
+	defer m.mu.Unlock()
 	m.menuItems = append(m.menuItems, "---")
 }
 
 func (m *MockSystray) SetTitle(title string) {
+	m.mu.Lock()
+	defer m.mu.Unlock()
 	m.title = title
 }
 

cmd/goose/ui.go

@@ -555,6 +555,10 @@ func (app *App) generatePRSectionTitles(prs []PR, sectionTitle string, hiddenOrg
 
 // rebuildMenu completely rebuilds the menu from scratch.
 func (app *App) rebuildMenu(ctx context.Context) {
+	// Prevent concurrent menu rebuilds
+	app.menuMutex.Lock()
+	defer app.menuMutex.Unlock()
+
 	// Rebuild entire menu
 	slog.Info("[MENU] Starting rebuildMenu", "os", runtime.GOOS)