diff --git a/Makefile b/Makefile index 4f0e327..784cf19 100644 --- a/Makefile +++ b/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..." diff --git a/README.md b/README.md index 150c4fb..31a1888 100644 --- a/README.md +++ b/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 diff --git a/github.go b/github.go index d647fbf..f16c286 100644 --- a/github.go +++ b/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") } } } diff --git a/go.mod b/go.mod index 6f9efcf..6e5d5d2 100644 --- a/go.mod +++ b/go.mod @@ -1,4 +1,4 @@ -module ready-to-review +module github.com/ready-to-review/pr-menubar go 1.23.4 @@ -6,6 +6,7 @@ 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 diff --git a/go.sum b/go.sum index ab599d3..fa45e7e 100644 --- a/go.sum +++ b/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= diff --git a/loginitem_darwin.go b/loginitem_darwin.go index 3fe377d..1537146 100644 --- a/loginitem_darwin.go +++ b/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 } diff --git a/main.go b/main.go index c156933..425ed19 100644 --- a/main.go +++ b/main.go @@ -344,72 +344,13 @@ func (app *App) updatePRs(ctx context.Context) { } // Update health status on success - log.Printf("[UPDATE] Successfully fetched %d incoming, %d outgoing PRs", len(incoming), len(outgoing)) app.mu.Lock() app.lastSuccessfulFetch = time.Now() app.consecutiveFailures = 0 app.mu.Unlock() - // Check for newly blocked PRs and send notifications - // Use a single lock for all operations on previousBlockedPRs and initialLoadComplete - app.mu.Lock() - oldBlockedPRs := app.previousBlockedPRs - initialLoadComplete := app.initialLoadComplete - app.mu.Unlock() - - log.Printf("[NOTIFY] Checking for newly blocked PRs (initialLoadComplete=%v, oldBlockedCount=%d)", initialLoadComplete, len(oldBlockedPRs)) - - currentBlockedPRs := make(map[string]bool) - var incomingBlocked, outgoingBlocked int - - // Count blocked PRs and send notifications - for i := range incoming { - if incoming[i].NeedsReview { - currentBlockedPRs[incoming[i].URL] = true - if !app.hideStaleIncoming || !incoming[i].UpdatedAt.Before(time.Now().Add(-stalePRThreshold)) { - incomingBlocked++ - } - // Send notification and play sound if PR wasn't blocked before - // (only after initial load to avoid startup noise) - if initialLoadComplete && !oldBlockedPRs[incoming[i].URL] { - log.Printf("[NOTIFY] Sending notification for newly blocked incoming PR: %s #%d", incoming[i].Repository, incoming[i].Number) - if err := beeep.Notify("PR Blocked on You", - fmt.Sprintf("%s #%d: %s", incoming[i].Repository, incoming[i].Number, incoming[i].Title), ""); err != nil { - log.Printf("Failed to send notification: %v", err) - } - app.playSound(ctx, "detective") - } else { - log.Printf("[NOTIFY] Skipping notification for incoming %s: initialLoadComplete=%v, wasBlocked=%v", - incoming[i].Repository, initialLoadComplete, oldBlockedPRs[incoming[i].URL]) - } - } - } - - for i := range outgoing { - if outgoing[i].IsBlocked { - currentBlockedPRs[outgoing[i].URL] = true - if !app.hideStaleIncoming || !outgoing[i].UpdatedAt.Before(time.Now().Add(-stalePRThreshold)) { - outgoingBlocked++ - } - // Send notification and play sound if PR wasn't blocked before - // (only after initial load to avoid startup noise) - if initialLoadComplete && !oldBlockedPRs[outgoing[i].URL] { - log.Printf("[NOTIFY] Sending notification for newly blocked outgoing PR: %s #%d", outgoing[i].Repository, outgoing[i].Number) - if err := beeep.Notify("PR Blocked on You", - fmt.Sprintf("%s #%d: %s", outgoing[i].Repository, outgoing[i].Number, outgoing[i].Title), ""); err != nil { - log.Printf("Failed to send notification: %v", err) - } - app.playSound(ctx, "rocket") - } else { - log.Printf("[NOTIFY] Skipping notification for outgoing %s: initialLoadComplete=%v, wasBlocked=%v", - outgoing[i].Repository, initialLoadComplete, oldBlockedPRs[outgoing[i].URL]) - } - } - } - // Update state atomically app.mu.Lock() - app.previousBlockedPRs = currentBlockedPRs app.incoming = incoming app.outgoing = outgoing // Mark initial load as complete after first successful update @@ -418,6 +359,9 @@ func (app *App) updatePRs(ctx context.Context) { } app.mu.Unlock() + // Don't check for newly blocked PRs here - wait for Turn data + // Turn data will be applied asynchronously and will trigger the check + app.updateMenuIfChanged(ctx) } @@ -488,34 +432,16 @@ func (app *App) updateMenuIfChanged(ctx context.Context) { log.Print("[MENU] Skipping menu update - Turn data still loading") return } - log.Print("[MENU] *** updateMenuIfChanged called - calculating diff ***") currentMenuState := app.buildCurrentMenuState() lastMenuState := app.lastMenuState - if lastMenuState == nil { - log.Print("[MENU] *** lastMenuState is NIL - will do initial build ***") - } else { - log.Printf("[MENU] *** lastMenuState exists (incoming:%d, outgoing:%d) - will compare ***", - len(lastMenuState.IncomingItems), len(lastMenuState.OutgoingItems)) - } app.mu.RUnlock() - if lastMenuState != nil { - diff := cmp.Diff(lastMenuState, currentMenuState) - log.Printf("[MENU] *** DIFF CALCULATION RESULT ***:\n%s", diff) - if diff == "" { - log.Printf("[MENU] Menu state unchanged, skipping update (incoming:%d, outgoing:%d)", - len(currentMenuState.IncomingItems), len(currentMenuState.OutgoingItems)) - return - } - log.Print("[MENU] *** DIFF DETECTED *** Menu state changed, rebuilding menu") - } else { - log.Printf("[MENU] Initial menu build (incoming:%d, outgoing:%d)", - len(currentMenuState.IncomingItems), len(currentMenuState.OutgoingItems)) + // Only rebuild if menu changed + if lastMenuState != nil && cmp.Diff(lastMenuState, currentMenuState) == "" { + return } app.mu.Lock() - log.Printf("[MENU] *** SAVING menu state (incoming:%d, outgoing:%d) ***", - len(currentMenuState.IncomingItems), len(currentMenuState.OutgoingItems)) app.lastMenuState = currentMenuState app.mu.Unlock() app.rebuildMenu(ctx) @@ -557,101 +483,137 @@ func (app *App) updatePRsWithWait(ctx context.Context) { // Still create initial menu even on error if !app.menuInitialized { - log.Println("Creating initial menu despite error") - log.Print("[MENU] Initializing menu structure") + // Create initial menu despite error app.rebuildMenu(ctx) app.menuInitialized = true - log.Print("[MENU] Menu initialization complete") + // Menu initialization complete } return } // Update health status on success - log.Printf("[UPDATE] Successfully fetched %d incoming, %d outgoing PRs", len(incoming), len(outgoing)) app.mu.Lock() app.lastSuccessfulFetch = time.Now() app.consecutiveFailures = 0 app.mu.Unlock() - // Check for newly blocked PRs and send notifications - // Use a single lock for all operations on previousBlockedPRs and initialLoadComplete + // Update state + app.mu.Lock() + app.incoming = incoming + app.outgoing = outgoing + app.mu.Unlock() + + // Create initial menu after first successful data load + if !app.menuInitialized { + // Create initial menu with Turn data + // Initialize menu structure + app.rebuildMenu(ctx) + app.menuInitialized = true + // Menu initialization complete + } else { + app.updateMenuIfChanged(ctx) + } + + // Mark initial load as complete after first successful update + if !app.initialLoadComplete { + app.mu.Lock() + app.initialLoadComplete = true + app.mu.Unlock() + } + // Check for newly blocked PRs + app.checkForNewlyBlockedPRs(ctx) +} + +// checkForNewlyBlockedPRs checks for PRs that have become blocked and sends notifications. +func (app *App) checkForNewlyBlockedPRs(ctx context.Context) { app.mu.Lock() oldBlockedPRs := app.previousBlockedPRs + if oldBlockedPRs == nil { + oldBlockedPRs = make(map[string]bool) + } initialLoadComplete := app.initialLoadComplete + hideStaleIncoming := app.hideStaleIncoming + incoming := make([]PR, len(app.incoming)) + copy(incoming, app.incoming) + outgoing := make([]PR, len(app.outgoing)) + copy(outgoing, app.outgoing) app.mu.Unlock() - log.Printf("[NOTIFY] Checking for newly blocked PRs (initialLoadComplete=%v, oldBlockedCount=%d)", initialLoadComplete, len(oldBlockedPRs)) + // Only log when we're checking after initial load is complete + if initialLoadComplete && len(oldBlockedPRs) > 0 { + log.Printf("[NOTIFY] Checking for newly blocked PRs (oldBlockedCount=%d)", len(oldBlockedPRs)) + } + + // Calculate stale threshold + now := time.Now() + staleThreshold := now.Add(-stalePRThreshold) currentBlockedPRs := make(map[string]bool) - var incomingBlocked, outgoingBlocked int + playedIncomingSound := false + playedOutgoingSound := false - // Count blocked PRs and send notifications + // Check incoming PRs for newly blocked ones for i := range incoming { if incoming[i].NeedsReview { currentBlockedPRs[incoming[i].URL] = true - if !app.hideStaleIncoming || !incoming[i].UpdatedAt.Before(time.Now().Add(-stalePRThreshold)) { - incomingBlocked++ + + // Skip stale PRs for notifications if hideStaleIncoming is enabled + if hideStaleIncoming && incoming[i].UpdatedAt.Before(staleThreshold) { + continue } + // Send notification and play sound if PR wasn't blocked before - // (only after initial load to avoid startup noise) - if initialLoadComplete && !oldBlockedPRs[incoming[i].URL] { - log.Printf("[NOTIFY] Sending notification for newly blocked incoming PR: %s #%d", incoming[i].Repository, incoming[i].Number) - if err := beeep.Notify("PR Blocked on You", - fmt.Sprintf("%s #%d: %s", incoming[i].Repository, incoming[i].Number, incoming[i].Title), ""); err != nil { - log.Printf("Failed to send notification: %v", err) + if !oldBlockedPRs[incoming[i].URL] { + log.Printf("[NOTIFY] New blocked incoming PR: %s #%d - %s (reason: %s)", + incoming[i].Repository, incoming[i].Number, incoming[i].Title, incoming[i].ActionReason) + title := "PR Blocked on You 🪿" + message := fmt.Sprintf("%s #%d: %s", incoming[i].Repository, incoming[i].Number, incoming[i].Title) + if err := beeep.Notify(title, message, ""); err != nil { + log.Printf("[NOTIFY] Failed to send desktop notification for %s: %v", incoming[i].URL, err) + } else { + log.Printf("[NOTIFY] Desktop notification sent for %s", incoming[i].URL) + } + // Only play sound once per polling period + if !playedIncomingSound { + app.playSound(ctx, "honk") + playedIncomingSound = true } - app.playSound(ctx, "detective") - } else { - log.Printf("[NOTIFY] Skipping notification for incoming %s: initialLoadComplete=%v, wasBlocked=%v", - incoming[i].Repository, initialLoadComplete, oldBlockedPRs[incoming[i].URL]) } } } + // Check outgoing PRs for newly blocked ones for i := range outgoing { if outgoing[i].IsBlocked { currentBlockedPRs[outgoing[i].URL] = true - if !app.hideStaleIncoming || !outgoing[i].UpdatedAt.Before(time.Now().Add(-stalePRThreshold)) { - outgoingBlocked++ + + // Skip stale PRs for notifications if hideStaleIncoming is enabled + if hideStaleIncoming && outgoing[i].UpdatedAt.Before(staleThreshold) { + continue } + // Send notification and play sound if PR wasn't blocked before - // (only after initial load to avoid startup noise) - if initialLoadComplete && !oldBlockedPRs[outgoing[i].URL] { - log.Printf("[NOTIFY] Sending notification for newly blocked outgoing PR: %s #%d", outgoing[i].Repository, outgoing[i].Number) - if err := beeep.Notify("PR Blocked on You", - fmt.Sprintf("%s #%d: %s", outgoing[i].Repository, outgoing[i].Number, outgoing[i].Title), ""); err != nil { - log.Printf("Failed to send notification: %v", err) + if !oldBlockedPRs[outgoing[i].URL] { + log.Printf("[NOTIFY] New blocked outgoing PR: %s #%d - %s (reason: %s)", + outgoing[i].Repository, outgoing[i].Number, outgoing[i].Title, outgoing[i].ActionReason) + title := "Your PR is Blocked 🚀" + message := fmt.Sprintf("%s #%d: %s", outgoing[i].Repository, outgoing[i].Number, outgoing[i].Title) + if err := beeep.Notify(title, message, ""); err != nil { + log.Printf("[NOTIFY] Failed to send desktop notification for %s: %v", outgoing[i].URL, err) + } else { + log.Printf("[NOTIFY] Desktop notification sent for %s", outgoing[i].URL) + } + // Only play sound once per polling period + if !playedOutgoingSound { + app.playSound(ctx, "rocket") + playedOutgoingSound = true } - app.playSound(ctx, "rocket") - } else { - log.Printf("[NOTIFY] Skipping notification for outgoing %s: initialLoadComplete=%v, wasBlocked=%v", - outgoing[i].Repository, initialLoadComplete, oldBlockedPRs[outgoing[i].URL]) } } } - // Update state + // Update the previous blocked PRs map app.mu.Lock() app.previousBlockedPRs = currentBlockedPRs - app.incoming = incoming - app.outgoing = outgoing app.mu.Unlock() - - // Create initial menu after first successful data load - if !app.menuInitialized { - log.Println("Creating initial menu with Turn data") - log.Print("[MENU] Initializing menu structure") - app.rebuildMenu(ctx) - app.menuInitialized = true - log.Print("[MENU] Menu initialization complete") - } else { - app.updateMenuIfChanged(ctx) - } - - // Mark initial load as complete after first successful update - if !app.initialLoadComplete { - app.mu.Lock() - app.initialLoadComplete = true - app.mu.Unlock() - } } diff --git a/media/423346__kinoton__canada-geese-honks.wav b/media/423346__kinoton__canada-geese-honks.wav new file mode 100644 index 0000000..52b7dd5 Binary files /dev/null and b/media/423346__kinoton__canada-geese-honks.wav differ diff --git a/media/honk.wav b/media/honk.wav new file mode 100644 index 0000000..ff4f650 Binary files /dev/null and b/media/honk.wav differ diff --git a/sound.go b/sound.go index 6462603..d8c4249 100644 --- a/sound.go +++ b/sound.go @@ -21,6 +21,9 @@ var launchSound []byte //go:embed media/dark-impact-232945.wav var impactSound []byte +//go:embed media/honk.wav +var honkSound []byte + var soundCacheOnce sync.Once // initSoundCache writes embedded sounds to cache directory once. @@ -48,6 +51,14 @@ func (app *App) initSoundCache() { log.Printf("Failed to cache impact sound: %v", err) } } + + // Write honk sound + honkPath := filepath.Join(soundDir, "honk.wav") + if _, err := os.Stat(honkPath); os.IsNotExist(err) { + if err := os.WriteFile(honkPath, honkSound, 0o600); err != nil { + log.Printf("Failed to cache honk sound: %v", err) + } + } }) } @@ -61,6 +72,7 @@ func (app *App) playSound(ctx context.Context, soundType string) { allowedSounds := map[string]string{ "rocket": "launch.wav", "detective": "impact.wav", + "honk": "honk.wav", } soundName, ok := allowedSounds[soundType] @@ -94,6 +106,17 @@ func (app *App) playSound(ctx context.Context, soundType string) { case "darwin": cmd = exec.CommandContext(soundCtx, "afplay", soundPath) case "windows": + // Validate soundPath contains only safe characters for PowerShell + // Allow alphanumeric, spaces, dots, underscores, hyphens, backslashes, and colons (for drive letters) + for _, r := range soundPath { + isValid := (r >= 'a' && r <= 'z') || (r >= 'A' && r <= 'Z') || + (r >= '0' && r <= '9') || r == ' ' || r == '.' || + r == '_' || r == '-' || r == '\\' || r == ':' + if !isValid { + log.Printf("Sound path contains invalid character for PowerShell: %q in path %s", r, soundPath) + return + } + } // Use PowerShell's SoundPlayer with proper escaping //nolint:gocritic // Need literal quotes in PowerShell script script := fmt.Sprintf(`(New-Object Media.SoundPlayer "%s").PlaySync()`, diff --git a/ui.go b/ui.go index 8821f28..e2deed2 100644 --- a/ui.go +++ b/ui.go @@ -144,9 +144,9 @@ func (app *App) setTrayTitle() { case counts.IncomingBlocked == 0 && counts.OutgoingBlocked == 0: systray.SetTitle("😊") case counts.IncomingBlocked > 0 && counts.OutgoingBlocked > 0: - systray.SetTitle(fmt.Sprintf("👀 %d 🎉 %d", counts.IncomingBlocked, counts.OutgoingBlocked)) + systray.SetTitle(fmt.Sprintf("🪿 %d 🎉 %d", counts.IncomingBlocked, counts.OutgoingBlocked)) case counts.IncomingBlocked > 0: - systray.SetTitle(fmt.Sprintf("👀 %d", counts.IncomingBlocked)) + systray.SetTitle(fmt.Sprintf("🪿 %d", counts.IncomingBlocked)) default: systray.SetTitle(fmt.Sprintf("🎉 %d", counts.OutgoingBlocked)) } @@ -183,7 +183,7 @@ func (app *App) addPRSection(ctx context.Context, prs []PR, sectionTitle string, // Add header headerText := fmt.Sprintf("%s — %d blocked on you", sectionTitle, blockedCount) - log.Printf("[MENU] Creating section header '%s': '%s'", sectionTitle, headerText) + // Create section header header := systray.AddMenuItem(headerText, "") header.Disable() @@ -210,7 +210,7 @@ func (app *App) addPRSection(ctx context.Context, prs []PR, sectionTitle string, tooltip = fmt.Sprintf("%s - %s", tooltip, sortedPRs[i].ActionReason) } - log.Printf("[MENU] Creating PR menu item for %s: '%s'", sortedPRs[i].URL, title) + // Create PR menu item item := systray.AddMenuItem(title, tooltip) // Capture URL to avoid loop variable capture bug @@ -225,7 +225,7 @@ func (app *App) addPRSection(ctx context.Context, prs []PR, sectionTitle string, // rebuildMenu completely rebuilds the menu from scratch. func (app *App) rebuildMenu(ctx context.Context) { - log.Print("[MENU] Rebuilding entire menu") + // Rebuild entire menu // Clear all existing menu items systray.ResetMenu() @@ -234,7 +234,7 @@ func (app *App) rebuildMenu(ctx context.Context) { app.setTrayTitle() // Dashboard at the top - log.Print("[MENU] Adding 'Dashboard' menu item at top") + // Add Dashboard link dashboardItem := systray.AddMenuItem("Dashboard", "") dashboardItem.Click(func() { if err := openURL(ctx, "https://dash.ready-to-review.dev/"); err != nil { @@ -249,7 +249,7 @@ func (app *App) rebuildMenu(ctx context.Context) { // Handle "No pull requests" case if counts.IncomingTotal == 0 && counts.OutgoingTotal == 0 { - log.Print("[MENU] Creating 'No pull requests' item") + // No PRs to display noPRs := systray.AddMenuItem("No pull requests", "") noPRs.Disable() } else { @@ -275,17 +275,17 @@ func (app *App) rebuildMenu(ctx context.Context) { // Add static items at the end app.addStaticMenuItems(ctx) - log.Print("[MENU] Menu rebuild complete") + // Menu rebuild complete } // addStaticMenuItems adds the static menu items (hide stale, start at login, quit). func (app *App) addStaticMenuItems(ctx context.Context) { - log.Print("[MENU] Adding static menu items") + // Add static menu items systray.AddSeparator() // Hide stale PRs - log.Print("[MENU] Adding 'Hide stale PRs' menu item") + // Add 'Hide stale PRs' option hideStaleItem := systray.AddMenuItem("Hide stale PRs (>90 days)", "") if app.hideStaleIncoming { hideStaleItem.Check() @@ -297,10 +297,10 @@ func (app *App) addStaticMenuItems(ctx context.Context) { } else { hideStaleItem.Uncheck() } - log.Printf("Hide stale PRs: %v", app.hideStaleIncoming) + // Toggle hide stale PRs setting // Force menu rebuild since hideStaleIncoming changed app.mu.Lock() - log.Print("[MENU] *** CLEARING menu state due to hide stale toggle ***") + // Clear menu state to force rebuild app.lastMenuState = nil app.mu.Unlock() app.rebuildMenu(ctx) @@ -310,7 +310,7 @@ func (app *App) addStaticMenuItems(ctx context.Context) { addLoginItemUI(ctx, app) // Quit - log.Print("[MENU] Adding 'Quit' menu item") + // Add 'Quit' option quitItem := systray.AddMenuItem("Quit", "") quitItem.Click(func() { log.Println("Quit requested by user")