Merge pull request #37 from codeGROOVE-dev/correct-highlighting

Fix bugs in how we highlight recently notified PRs

Author: tstromberg

cmd/goose/github.go

@@ -634,16 +634,12 @@ func (app *App) fetchTurnDataAsync(ctx context.Context, issues []*github.Issue,
 	}
 
 	// Only check for newly blocked PRs if there were actual changes
+	// checkForNewlyBlockedPRs will handle UI updates internally if needed
 	if actualChanges > 0 {
 		app.checkForNewlyBlockedPRs(ctx)
-	}
-
-	// Update tray title and menu with final Turn data if menu is already initialized
-	app.setTrayTitle()
-	if app.menuInitialized {
-		// Only trigger menu update if PR data actually changed
-		if actualChanges > 0 {
-			app.updateMenu(ctx)
-		}
+		// UI updates are handled inside checkForNewlyBlockedPRs
+	} else {
+		// No changes, but still update tray title in case of initial load
+		app.setTrayTitle()
 	}
 }

cmd/goose/main.go

@@ -568,15 +568,26 @@ func (app *App) notifyWithSound(ctx context.Context, pr PR, isIncoming bool, pla
 
 // checkForNewlyBlockedPRs sends notifications for blocked PRs.
 func (app *App) checkForNewlyBlockedPRs(ctx context.Context) {
-	app.mu.RLock()
-	incoming := app.incoming
-	outgoing := app.outgoing
+	// Check for context cancellation early
+	select {
+	case <-ctx.Done():
+		log.Print("[BLOCKED] Context cancelled, skipping newly blocked PR check")
+		return
+	default:
+	}
+
+	app.mu.Lock()
+	// Make deep copies to work with while holding the lock
+	incoming := make([]PR, len(app.incoming))
+	copy(incoming, app.incoming)
+	outgoing := make([]PR, len(app.outgoing))
+	copy(outgoing, app.outgoing)
 	previousBlocked := app.previousBlockedPRs
 	blockedTimes := app.blockedPRTimes
 	autoBrowserEnabled := app.enableAutoBrowser
 	startTime := app.startTime
 	hideStaleIncoming := app.hideStaleIncoming
-	app.mu.RUnlock()
+	app.mu.Unlock()
 
 	currentBlocked := make(map[string]bool)
 	newBlockedTimes := make(map[string]time.Time)
@@ -597,6 +608,8 @@ func (app *App) checkForNewlyBlockedPRs(ctx context.Context) {
 				// Newly blocked PR
 				newBlockedTimes[incoming[i].URL] = now
 				incoming[i].FirstBlockedAt = now
+				log.Printf("[BLOCKED] Setting FirstBlockedAt for incoming PR: %s #%d at %v",
+					incoming[i].Repository, incoming[i].Number, now)
 
 				// Skip sound and auto-open for stale PRs when hideStaleIncoming is enabled
 				isStale := incoming[i].UpdatedAt.Before(staleThreshold)
@@ -625,6 +638,8 @@ func (app *App) checkForNewlyBlockedPRs(ctx context.Context) {
 				// Newly blocked PR
 				newBlockedTimes[outgoing[i].URL] = now
 				outgoing[i].FirstBlockedAt = now
+				log.Printf("[BLOCKED] Setting FirstBlockedAt for outgoing PR: %s #%d at %v",
+					outgoing[i].Repository, outgoing[i].Number, now)
 
 				// Skip sound and auto-open for stale PRs when hideStaleIncoming is enabled
 				isStale := outgoing[i].UpdatedAt.Before(staleThreshold)
@@ -645,11 +660,21 @@ func (app *App) checkForNewlyBlockedPRs(ctx context.Context) {
 		}
 	}
 
+	// Update state with a lock
 	app.mu.Lock()
 	app.previousBlockedPRs = currentBlocked
 	app.blockedPRTimes = newBlockedTimes
 	// Update the PR lists with FirstBlockedAt times
 	app.incoming = incoming
 	app.outgoing = outgoing
+	menuInitialized := app.menuInitialized
 	app.mu.Unlock()
+
+	// Update UI after releasing the lock
+	// Only update if there are newly blocked PRs
+	if menuInitialized && len(currentBlocked) > len(previousBlocked) {
+		log.Print("[BLOCKED] Updating UI for newly blocked PRs")
+		app.setTrayTitle()
+		app.updateMenu(ctx)
+	}
 }

cmd/goose/ui.go

@@ -142,16 +142,21 @@ func (app *App) setTrayTitle() {
 	counts := app.countPRs()
 
 	// Set title based on PR state
+	var title string
 	switch {
 	case counts.IncomingBlocked == 0 && counts.OutgoingBlocked == 0:
-		systray.SetTitle("😊")
+		title = "😊"
 	case counts.IncomingBlocked > 0 && counts.OutgoingBlocked > 0:
-		systray.SetTitle(fmt.Sprintf("🪿 %d 🎉 %d", counts.IncomingBlocked, counts.OutgoingBlocked))
+		title = fmt.Sprintf("🪿 %d 🎉 %d", counts.IncomingBlocked, counts.OutgoingBlocked)
 	case counts.IncomingBlocked > 0:
-		systray.SetTitle(fmt.Sprintf("🪿 %d", counts.IncomingBlocked))
+		title = fmt.Sprintf("🪿 %d", counts.IncomingBlocked)
 	default:
-		systray.SetTitle(fmt.Sprintf("🎉 %d", counts.OutgoingBlocked))
+		title = fmt.Sprintf("🎉 %d", counts.OutgoingBlocked)
 	}
+
+	log.Printf("[TRAY] Setting title: %s (incoming_blocked=%d, outgoing_blocked=%d)",
+		title, counts.IncomingBlocked, counts.OutgoingBlocked)
+	systray.SetTitle(title)
 }
 
 // sortPRsBlockedFirst creates a sorted copy of PRs with blocked ones first.
@@ -201,11 +206,20 @@ func (app *App) addPRSection(ctx context.Context, prs []PR, sectionTitle string,
 		}
 
 		title := fmt.Sprintf("%s #%d", sortedPRs[i].Repository, sortedPRs[i].Number)
-		// Add bullet point or goose for blocked PRs
+		// Add bullet point or emoji for blocked PRs
 		if sortedPRs[i].NeedsReview || sortedPRs[i].IsBlocked {
-			// Show goose emoji for PRs blocked within the last hour
+			// Show emoji for PRs blocked within the last hour
 			if !sortedPRs[i].FirstBlockedAt.IsZero() && time.Since(sortedPRs[i].FirstBlockedAt) < time.Hour {
-				title = fmt.Sprintf("🪿 %s", title)
+				// Use party popper for outgoing PRs, goose for incoming PRs
+				if sectionTitle == "Outgoing" {
+					title = fmt.Sprintf("🎉 %s", title)
+					log.Printf("[MENU] Adding party popper to outgoing PR: %s (blocked %v ago)",
+						sortedPRs[i].URL, time.Since(sortedPRs[i].FirstBlockedAt))
+				} else {
+					title = fmt.Sprintf("🪿 %s", title)
+					log.Printf("[MENU] Adding goose to incoming PR: %s (blocked %v ago)",
+						sortedPRs[i].URL, time.Since(sortedPRs[i].FirstBlockedAt))
+				}
 			} else {
 				title = fmt.Sprintf("• %s", title)
 			}