Merge pull request #13 from ready-to-review/readme

Add more HONK to notifications system

Author: tstromberg

Makefile

@@ -126,6 +126,8 @@ app-bundle: out build-darwin install-appify
 		/usr/libexec/PlistBuddy -c "Set :LSUIElement true" "out/$(BUNDLE_NAME).app/Contents/Info.plist"
 	@/usr/libexec/PlistBuddy -c "Add :CFBundleDevelopmentRegion string en" "out/$(BUNDLE_NAME).app/Contents/Info.plist" 2>/dev/null || \
 		/usr/libexec/PlistBuddy -c "Set :CFBundleDevelopmentRegion en" "out/$(BUNDLE_NAME).app/Contents/Info.plist"
+	@/usr/libexec/PlistBuddy -c "Add :NSUserNotificationAlertStyle string alert" "out/$(BUNDLE_NAME).app/Contents/Info.plist" 2>/dev/null || \
+		/usr/libexec/PlistBuddy -c "Set :NSUserNotificationAlertStyle alert" "out/$(BUNDLE_NAME).app/Contents/Info.plist"
 
 	# Remove extended attributes and code sign the app bundle
 	@echo "Preparing app bundle for signing..."

README.md

@@ -16,17 +16,18 @@ git clone https://github.com/ready-to-review/pr-menubar.git
 cd pr-menubar && make run
 ```
 
-The app appears in your menubar showing: `incoming / outgoing` PRs
+The app appears in your menubar showing: 🪿 (incoming blocked on you) or 🎉 (outgoing blocked)
 
 ## Features
 
-- **Smart Notifications**: Only alerts when YOU are the blocker (not just assigned)
-- **Sound Effects**: Audio cues for important PR events 🔊
+- **Smart Notifications**: Desktop alerts + sounds when PRs become blocked (🪿 honk for incoming, 🚀 rocket for outgoing)
+- **Comprehensive Coverage**: Tracks PRs you're involved in + PRs in your repos needing reviewers
 - **Detailed Tooltips**: Hover to see why you're blocking and what's needed
 - **Test-Aware**: Waits for CI to pass before notifying
 - **Zero Noise**: No pings for PRs that aren't actually blocked on you
 - **One-Click Access**: Open any PR instantly from the menubar
 - **Multi-User Support**: Track PRs for different GitHub accounts with `--user`
+- **Auto-Start**: macOS "Start at Login" option (when running from /Applications)
 
 ## Installation
 
@@ -36,7 +37,7 @@ make install      # Traditional install for your OS
 make build        # Build only
 ```
 
-**Requirements**: GitHub CLI (`gh`) authenticated, Go 1.21+ (for building)
+**Requirements**: GitHub CLI (`gh`) authenticated, Go 1.23+ (for building)
 
 ## Privacy
 

github.go

@@ -119,32 +119,12 @@ func (*App) githubToken(ctx context.Context) (string, error) {
 	return token, nil
 }
 
-// fetchPRsInternal is the implementation for PR fetching.
-// It returns GitHub data immediately and starts Turn API queries in the background (when waitForTurn=false),
-// or waits for Turn data to complete (when waitForTurn=true).
-func (app *App) fetchPRsInternal(ctx context.Context, waitForTurn bool) (incoming []PR, outgoing []PR, err error) {
-	// Use targetUser if specified, otherwise use authenticated user
-	user := app.currentUser.GetLogin()
-	if app.targetUser != "" {
-		user = app.targetUser
-	}
-
-	// Single query to get all PRs involving the user
-	query := fmt.Sprintf("is:open is:pr involves:%s archived:false", user)
-
-	const perPage = 100
-	opts := &github.SearchOptions{
-		ListOptions: github.ListOptions{PerPage: perPage},
-		Sort:        "updated",
-		Order:       "desc",
-	}
-
-	log.Printf("Searching for PRs with query: %s", query)
-	searchStart := time.Now()
-
+// executeGitHubQuery executes a single GitHub search query with retry logic.
+func (app *App) executeGitHubQuery(ctx context.Context, query string, opts *github.SearchOptions) (*github.IssuesSearchResult, error) {
 	var result *github.IssuesSearchResult
 	var resp *github.Response
-	err = retry.Do(func() error {
+
+	err := retry.Do(func() error {
 		// Create timeout context for GitHub API call
 		githubCtx, cancel := context.WithTimeout(ctx, 30*time.Second)
 		defer cancel()
@@ -194,19 +174,97 @@ func (app *App) fetchPRsInternal(ctx context.Context, waitForTurn bool) (incomin
 		retry.Context(ctx),
 	)
 	if err != nil {
-		return nil, nil, fmt.Errorf("search PRs after %d retries: %w", maxRetries, err)
+		return nil, err
 	}
+	return result, nil
+}
 
-	log.Printf("GitHub search completed in %v, found %d PRs", time.Since(searchStart), len(result.Issues))
+// fetchPRsInternal is the implementation for PR fetching.
+// It returns GitHub data immediately and starts Turn API queries in the background (when waitForTurn=false),
+// or waits for Turn data to complete (when waitForTurn=true).
+func (app *App) fetchPRsInternal(ctx context.Context, waitForTurn bool) (incoming []PR, outgoing []PR, err error) {
+	// Use targetUser if specified, otherwise use authenticated user
+	user := app.currentUser.GetLogin()
+	if app.targetUser != "" {
+		user = app.targetUser
+	}
+
+	const perPage = 100
+	opts := &github.SearchOptions{
+		ListOptions: github.ListOptions{PerPage: perPage},
+		Sort:        "updated",
+		Order:       "desc",
+	}
+
+	searchStart := time.Now()
+
+	// Run both queries in parallel
+	type queryResult struct {
+		query  string
+		issues []*github.Issue
+		err    error
+	}
+
+	queryResults := make(chan queryResult, 2)
+
+	// Query 1: PRs involving the user
+	go func() {
+		query := fmt.Sprintf("is:open is:pr involves:%s archived:false", user)
+		log.Printf("[GITHUB] Searching for PRs with query: %s", query)
+
+		result, err := app.executeGitHubQuery(ctx, query, opts)
+		if err != nil {
+			queryResults <- queryResult{err: err, query: query}
+		} else {
+			queryResults <- queryResult{issues: result.Issues, query: query}
+		}
+	}()
+
+	// Query 2: PRs in user-owned repos with no reviewers
+	go func() {
+		query := fmt.Sprintf("is:open is:pr user:%s review:none archived:false", user)
+		log.Printf("[GITHUB] Searching for PRs with query: %s", query)
+
+		result, err := app.executeGitHubQuery(ctx, query, opts)
+		if err != nil {
+			queryResults <- queryResult{err: err, query: query}
+		} else {
+			queryResults <- queryResult{issues: result.Issues, query: query}
+		}
+	}()
+
+	// Collect results from both queries
+	var allIssues []*github.Issue
+	seenURLs := make(map[string]bool)
+
+	for range 2 {
+		result := <-queryResults
+		if result.err != nil {
+			log.Printf("[GITHUB] Query failed: %s - %v", result.query, result.err)
+			// Continue processing other query results even if one fails
+			continue
+		}
+		log.Printf("[GITHUB] Query completed: %s - found %d PRs", result.query, len(result.issues))
+		
+		// Deduplicate PRs based on URL
+		for _, issue := range result.issues {
+			url := issue.GetHTMLURL()
+			if !seenURLs[url] {
+				seenURLs[url] = true
+				allIssues = append(allIssues, issue)
+			}
+		}
+	}
+	log.Printf("[GITHUB] Both searches completed in %v, found %d unique PRs", time.Since(searchStart), len(allIssues))
 
 	// Limit PRs for performance
-	if len(result.Issues) > maxPRsToProcess {
-		log.Printf("Limiting to %d PRs for performance (total: %d)", maxPRsToProcess, len(result.Issues))
-		result.Issues = result.Issues[:maxPRsToProcess]
+	if len(allIssues) > maxPRsToProcess {
+		log.Printf("Limiting to %d PRs for performance (total: %d)", maxPRsToProcess, len(allIssues))
+		allIssues = allIssues[:maxPRsToProcess]
 	}
 
 	// Process GitHub results immediately
-	for _, issue := range result.Issues {
+	for _, issue := range allIssues {
 		if !issue.IsPullRequest() {
 			continue
 		}
@@ -229,26 +287,21 @@ func (app *App) fetchPRsInternal(ctx context.Context, waitForTurn bool) (incomin
 		}
 	}
 
+	// Only log summary, not individual PRs
 	log.Printf("[GITHUB] Found %d incoming, %d outgoing PRs from GitHub", len(incoming), len(outgoing))
-	for i := range incoming {
-		log.Printf("[GITHUB] Incoming PR: %s", incoming[i].URL)
-	}
-	for i := range outgoing {
-		log.Printf("[GITHUB] Outgoing PR: %s", outgoing[i].URL)
-	}
 
 	// Fetch Turn API data
 	if waitForTurn {
 		// Synchronous - wait for Turn data
-		log.Println("[TURN] Fetching Turn API data synchronously before building menu...")
-		app.fetchTurnDataSync(ctx, result.Issues, user, &incoming, &outgoing)
+		// Fetch Turn API data synchronously before building menu
+		app.fetchTurnDataSync(ctx, allIssues, user, &incoming, &outgoing)
 	} else {
 		// Asynchronous - start in background
 		app.mu.Lock()
 		app.loadingTurnData = true
 		app.pendingTurnResults = make([]TurnResult, 0) // Reset buffer
 		app.mu.Unlock()
-		go app.fetchTurnDataAsync(ctx, result.Issues, user)
+		go app.fetchTurnDataAsync(ctx, allIssues, user)
 	}
 
 	return incoming, outgoing, nil
@@ -366,7 +419,10 @@ func (app *App) fetchTurnDataSync(ctx context.Context, issues []*github.Issue, u
 			if action, exists := result.turnData.PRState.UnblockAction[user]; exists {
 				needsReview = true
 				actionReason = action.Reason
-				log.Printf("[TURN] UnblockAction for %s: Reason=%q, Kind=%q", result.url, action.Reason, action.Kind)
+				// Only log fresh API calls
+				if !result.wasFromCache {
+					log.Printf("[TURN] UnblockAction for %s: Reason=%q, Kind=%q", result.url, action.Reason, action.Kind)
+				}
 			}
 
 			// Update the PR in the slices directly
@@ -400,7 +456,7 @@ func (app *App) fetchTurnDataSync(ctx context.Context, issues []*github.Issue, u
 // fetchTurnDataAsync fetches Turn API data in the background and updates PRs as results arrive.
 func (app *App) fetchTurnDataAsync(ctx context.Context, issues []*github.Issue, user string) {
 	// Log start of Turn API queries
-	log.Print("[TURN] Starting Turn API queries in background")
+	// Start Turn API queries in background
 
 	turnStart := time.Now()
 	type prResult struct {
@@ -466,10 +522,10 @@ func (app *App) fetchTurnDataAsync(ctx context.Context, issues []*github.Issue,
 			if action, exists := result.turnData.PRState.UnblockAction[user]; exists {
 				needsReview = true
 				actionReason = action.Reason
-				log.Printf("[TURN] UnblockAction for %s: Reason=%q, Kind=%q", result.url, action.Reason, action.Kind)
-			} else if !result.wasFromCache {
-				// Only log "no action" for fresh API results, not cached ones
-				log.Printf("[TURN] No UnblockAction found for user %s on %s", user, result.url)
+				// Only log blocked PRs from fresh API calls
+				if !result.wasFromCache {
+					log.Printf("[TURN] UnblockAction for %s: Reason=%q, Kind=%q", result.url, action.Reason, action.Kind)
+				}
 			}
 
 			// Buffer the Turn result instead of applying immediately
@@ -486,13 +542,9 @@ func (app *App) fetchTurnDataAsync(ctx context.Context, issues []*github.Issue,
 			app.mu.Unlock()
 
 			updatesApplied++
-			// Reduce verbosity - only log if not from cache or if blocked
-			if !result.wasFromCache || needsReview {
-				cacheStatus := "fresh"
-				if result.wasFromCache {
-					cacheStatus = "cached"
-				}
-				log.Printf("[TURN] %s data for %s (needsReview=%v, actionReason=%q)", cacheStatus, result.url, needsReview, actionReason)
+			// Only log fresh API calls (not cached)
+			if !result.wasFromCache {
+				log.Printf("[TURN] Fresh API data for %s (needsReview=%v)", result.url, needsReview)
 			}
 		} else if result.err != nil {
 			turnFailures++
@@ -519,7 +571,10 @@ func (app *App) fetchTurnDataAsync(ctx context.Context, issues []*github.Issue,
 		}
 	}
 
-	log.Printf("[TURN] Applying %d buffered Turn results (%d from cache, %d fresh)", len(pendingResults), cacheHits, freshResults)
+	// Only log if we have fresh results
+	if freshResults > 0 {
+		log.Printf("[TURN] Applying %d buffered Turn results (%d from cache, %d fresh)", len(pendingResults), cacheHits, freshResults)
+	}
 
 	// Track how many PRs actually changed
 	var actualChanges int
@@ -530,15 +585,15 @@ func (app *App) fetchTurnDataAsync(ctx context.Context, issues []*github.Issue,
 		}
 	}
 
+	// Check for newly blocked PRs after Turn data is applied
+	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 {
-			log.Printf("[TURN] Turn data applied - %d PRs actually changed, checking if menu needs update", actualChanges)
 			app.updateMenuIfChanged(ctx)
-		} else {
-			log.Print("[TURN] Turn data applied - no PR changes detected (cached data unchanged), skipping menu update")
 		}
 	}
 }

go.mod

@@ -1,11 +1,12 @@
-module ready-to-review
+module github.com/ready-to-review/pr-menubar
 
 go 1.23.4
 
 require (
 	github.com/codeGROOVE-dev/retry v1.2.0
 	github.com/energye/systray v1.0.2
 	github.com/gen2brain/beeep v0.11.1
+	github.com/google/go-cmp v0.7.0
 	github.com/google/go-github/v57 v57.0.0
 	github.com/ready-to-review/turnclient v0.0.0-20250718014946-bb5bb107649f
 	golang.org/x/oauth2 v0.30.0
@@ -16,7 +17,6 @@ require (
 	github.com/esiqveland/notify v0.13.3 // indirect
 	github.com/go-ole/go-ole v1.3.0 // indirect
 	github.com/godbus/dbus/v5 v5.1.0 // indirect
-	github.com/google/go-cmp v0.7.0 // indirect
 	github.com/google/go-querystring v1.1.0 // indirect
 	github.com/jackmordaunt/icns/v3 v3.0.1 // indirect
 	github.com/nfnt/resize v0.0.0-20180221191011-83c6a9932646 // indirect

go.sum

@@ -17,8 +17,6 @@ github.com/godbus/dbus/v5 v5.0.4/go.mod h1:xhWf0FNVPg57R7Z0UbKHbJfkEywrmjJnf7w5x
 github.com/godbus/dbus/v5 v5.1.0 h1:4KLkAxT3aOY8Li4FRJe/KvhoNFFxo0m6fNuFUO8QJUk=
 github.com/godbus/dbus/v5 v5.1.0/go.mod h1:xhWf0FNVPg57R7Z0UbKHbJfkEywrmjJnf7w5xrFpKfA=
 github.com/google/go-cmp v0.5.2/go.mod h1:v8dTdLbMG2kIc/vJvl+f65V22dbkXbowE6jgT/gNBxE=
-github.com/google/go-cmp v0.6.0 h1:ofyhxvXcZhMsU5ulbFiLKl/XBFqE1GSq7atu8tAmTRI=
-github.com/google/go-cmp v0.6.0/go.mod h1:17dUlkBOakJ0+DkrSSNjCkIjxS6bF9zb3elmeNGIjoY=
 github.com/google/go-cmp v0.7.0 h1:wk8382ETsv4JYUZwIsn6YpYiWiBsYLSJiTsyBybVuN8=
 github.com/google/go-cmp v0.7.0/go.mod h1:pXiqmnSA92OHEEa9HXL2W4E7lf9JzCmGVUdgjX3N/iU=
 github.com/google/go-github/v57 v57.0.0 h1:L+Y3UPTY8ALM8x+TV0lg+IEBI+upibemtBD8Q9u7zHs=

loginitem_darwin.go

@@ -113,24 +113,6 @@ func setLoginItem(ctx context.Context, enable bool) error {
 	return nil
 }
 
-// isRunningFromAppBundle checks if the app is running from a .app bundle.
-func isRunningFromAppBundle() bool {
-	execPath, err := os.Executable()
-	if err != nil {
-		return false
-	}
-
-	// Resolve any symlinks
-	execPath, err = filepath.EvalSymlinks(execPath)
-	if err != nil {
-		return false
-	}
-
-	// Check if we're running from an app bundle
-	// App bundles have the structure: /path/to/App.app/Contents/MacOS/executable
-	return strings.Contains(execPath, ".app/Contents/MacOS/")
-}
-
 // appPath returns the path to the application bundle.
 func appPath() (string, error) {
 	// Get the executable path
@@ -161,8 +143,22 @@ func appPath() (string, error) {
 
 // addLoginItemUI adds the login item menu option (macOS only).
 func addLoginItemUI(ctx context.Context, _ *App) {
-	// Only show login item menu if running from an app bundle
-	if !isRunningFromAppBundle() {
+	// Check if we're running from an app bundle
+	execPath, err := os.Executable()
+	if err != nil {
+		log.Println("Hiding 'Start at Login' menu item - could not get executable path")
+		return
+	}
+
+	// Resolve any symlinks
+	execPath, err = filepath.EvalSymlinks(execPath)
+	if err != nil {
+		log.Println("Hiding 'Start at Login' menu item - could not resolve symlinks")
+		return
+	}
+
+	// App bundles have the structure: /path/to/App.app/Contents/MacOS/executable
+	if !strings.Contains(execPath, ".app/Contents/MacOS/") {
 		log.Println("Hiding 'Start at Login' menu item - not running from app bundle")
 		return
 	}