From 5f3c067e7dcafe80d03546a206290cde95ab58aa Mon Sep 17 00:00:00 2001 From: Thomas Stromberg Date: Tue, 5 Aug 2025 19:46:40 -0400 Subject: [PATCH] prevent unnecessary menu builds --- github.go | 192 ++++++++++++--------------------------------- go.mod | 1 + go.sum | 2 + main.go | 218 +++++++++++++++++++++++++++++++++++++++------------ main_test.go | 35 --------- sound.go | 1 + ui.go | 5 +- 7 files changed, 230 insertions(+), 224 deletions(-) diff --git a/github.go b/github.go index 5be47fd..4cc8309 100644 --- a/github.go +++ b/github.go @@ -122,6 +122,16 @@ func (*App) githubToken(ctx context.Context) (string, error) { // fetchPRs retrieves all PRs involving the current user. // It returns GitHub data immediately and starts Turn API queries in the background. func (app *App) fetchPRs(ctx context.Context) (incoming []PR, outgoing []PR, err error) { + return app.fetchPRsInternal(ctx, false) +} + +// fetchPRsWithWait fetches PRs and waits for Turn data to complete. +func (app *App) fetchPRsWithWait(ctx context.Context) (incoming []PR, outgoing []PR, err error) { + return app.fetchPRsInternal(ctx, true) +} + +// fetchPRsInternal is the common implementation for PR fetching. +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 != "" { @@ -236,8 +246,19 @@ func (app *App) fetchPRs(ctx context.Context) (incoming []PR, outgoing []PR, err log.Printf("[GITHUB] Outgoing PR: %s", pr.URL) } - // Start Turn API queries in background - go app.fetchTurnDataAsync(ctx, result.Issues, user) + // 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) + } 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) + } return incoming, outgoing, nil } @@ -270,122 +291,6 @@ func (app *App) updatePRData(url string, needsReview bool, isOwner bool, actionR return nil } -// fetchPRsWithWait fetches PRs and waits for Turn data to complete. -func (app *App) fetchPRsWithWait(ctx context.Context) (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() - - var result *github.IssuesSearchResult - var resp *github.Response - err = retry.Do(func() error { - // Create timeout context for GitHub API call - githubCtx, cancel := context.WithTimeout(ctx, 30*time.Second) - defer cancel() - - var retryErr error - result, resp, retryErr = app.client.Search.Issues(githubCtx, query, opts) - if retryErr != nil { - // Enhanced error handling with specific cases - if resp != nil { - const ( - httpStatusUnauthorized = 401 - httpStatusForbidden = 403 - httpStatusUnprocessable = 422 - ) - switch resp.StatusCode { - case httpStatusForbidden: - if resp.Header.Get("X-Ratelimit-Remaining") == "0" { - resetTime := resp.Header.Get("X-Ratelimit-Reset") - log.Printf("GitHub API rate limited, reset at: %s (will retry)", resetTime) - return retryErr // Retry on rate limit - } - log.Print("GitHub API access forbidden (check token permissions)") - return retry.Unrecoverable(fmt.Errorf("github API access forbidden: %w", retryErr)) - case httpStatusUnauthorized: - log.Print("GitHub API authentication failed (check token)") - return retry.Unrecoverable(fmt.Errorf("github API authentication failed: %w", retryErr)) - case httpStatusUnprocessable: - log.Printf("GitHub API query invalid: %s", query) - return retry.Unrecoverable(fmt.Errorf("github API query invalid: %w", retryErr)) - default: - log.Printf("GitHub API error (status %d): %v (will retry)", resp.StatusCode, retryErr) - } - } else { - // Likely network error - retry these - log.Printf("GitHub API network error: %v (will retry)", retryErr) - } - return retryErr - } - return nil - }, - retry.Attempts(maxRetries), - retry.DelayType(retry.BackOffDelay), - retry.MaxDelay(maxRetryDelay), - retry.OnRetry(func(n uint, err error) { - log.Printf("GitHub Search.Issues retry %d/%d: %v", n+1, maxRetries, err) - }), - retry.Context(ctx), - ) - if err != nil { - return nil, nil, fmt.Errorf("search PRs after %d retries: %w", maxRetries, err) - } - - log.Printf("GitHub search completed in %v, found %d PRs", time.Since(searchStart), len(result.Issues)) - - // 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] - } - - // Process GitHub results - for _, issue := range result.Issues { - if !issue.IsPullRequest() { - continue - } - repo := strings.TrimPrefix(issue.GetRepositoryURL(), "https://api.github.com/repos/") - - pr := PR{ - Title: issue.GetTitle(), - URL: issue.GetHTMLURL(), - Repository: repo, - Number: issue.GetNumber(), - UpdatedAt: issue.GetUpdatedAt().Time, - } - - // Categorize as incoming or outgoing - if issue.GetUser().GetLogin() == user { - outgoing = append(outgoing, pr) - } else { - incoming = append(incoming, pr) - } - } - - log.Printf("[GITHUB] Found %d incoming, %d outgoing PRs from GitHub", len(incoming), len(outgoing)) - - // Now fetch Turn data synchronously - log.Println("[TURN] Fetching Turn API data synchronously before building menu...") - app.fetchTurnDataSync(ctx, result.Issues, user, &incoming, &outgoing) - - return incoming, outgoing, nil -} - // fetchTurnDataSync fetches Turn API data synchronously and updates PRs directly. func (app *App) fetchTurnDataSync(ctx context.Context, issues []*github.Issue, user string, incoming *[]PR, outgoing *[]PR) { turnStart := time.Now() @@ -533,11 +438,7 @@ func (app *App) fetchTurnDataAsync(ctx context.Context, issues []*github.Issue, turnFailures := 0 updatesApplied := 0 - // Batch updates to reduce menu rebuilds - updateBatch := 0 - const batchSize = 10 - lastUpdateTime := time.Now() - const minUpdateInterval = 500 * time.Millisecond + // Process results as they arrive and buffer them for result := range results { if result.err == nil && result.turnData != nil && result.turnData.PRState.UnblockAction != nil { @@ -554,22 +455,20 @@ func (app *App) fetchTurnDataAsync(ctx context.Context, issues []*github.Issue, log.Printf("[TURN] No UnblockAction found for user %s on %s", user, result.url) } - // Update the PR in our lists - pr := app.updatePRData(result.url, needsReview, result.isOwner, actionReason) + // Buffer the Turn result instead of applying immediately + turnResult := TurnResult{ + URL: result.url, + NeedsReview: needsReview, + IsOwner: result.isOwner, + ActionReason: actionReason, + } - if pr != nil { - updatesApplied++ - updateBatch++ - log.Printf("[TURN] Turn data received for %s (needsReview=%v, actionReason=%q)", result.url, needsReview, actionReason) + app.mu.Lock() + app.pendingTurnResults = append(app.pendingTurnResults, turnResult) + app.mu.Unlock() - // Periodically update tray title - if updateBatch >= batchSize || time.Since(lastUpdateTime) >= minUpdateInterval { - log.Printf("[TURN] Batch update threshold reached (%d updates), updating title", updateBatch) - app.setTrayTitle() - updateBatch = 0 - lastUpdateTime = time.Now() - } - } + updatesApplied++ + log.Printf("[TURN] Turn data received for %s (needsReview=%v, actionReason=%q) - buffered", result.url, needsReview, actionReason) } else if result.err != nil { turnFailures++ } @@ -578,9 +477,22 @@ func (app *App) fetchTurnDataAsync(ctx context.Context, issues []*github.Issue, log.Printf("[TURN] Turn API queries completed in %v (%d/%d succeeded, %d PRs updated)", time.Since(turnStart), turnSuccesses, turnSuccesses+turnFailures, updatesApplied) - // Rebuild menu with final Turn data if menu is already initialized + // Apply all buffered Turn results at once + app.mu.Lock() + pendingResults := app.pendingTurnResults + app.pendingTurnResults = nil + app.loadingTurnData = false + app.mu.Unlock() + + log.Printf("[TURN] Applying %d buffered Turn results", len(pendingResults)) + for _, result := range pendingResults { + app.updatePRData(result.URL, result.NeedsReview, result.IsOwner, result.ActionReason) + } + + // Update tray title and menu with final Turn data if menu is already initialized + app.setTrayTitle() if app.menuInitialized { - log.Print("[TURN] Turn data loaded, rebuilding menu") - app.rebuildMenu(ctx) + log.Print("[TURN] Turn data loaded, checking if menu needs update") + app.updateMenuIfChanged(ctx) } } diff --git a/go.mod b/go.mod index a2bf3ad..6f9efcf 100644 --- a/go.mod +++ b/go.mod @@ -16,6 +16,7 @@ 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 b8d3375..ab599d3 100644 --- a/go.sum +++ b/go.sum @@ -19,6 +19,8 @@ github.com/godbus/dbus/v5 v5.1.0/go.mod h1:xhWf0FNVPg57R7Z0UbKHbJfkEywrmjJnf7w5x 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= github.com/google/go-github/v57 v57.0.0/go.mod h1:s0omdnye0hvK/ecLvpsGfJMiRt85PimQh4oygmLIxHw= github.com/google/go-querystring v1.1.0 h1:AnCroh3fv4ZBgVIf1Iwtovgjaw/GiKJo8M8yD/fhyJ8= diff --git a/main.go b/main.go index f662302..65a5e6d 100644 --- a/main.go +++ b/main.go @@ -15,6 +15,8 @@ import ( "sync" "time" + "github.com/google/go-cmp/cmp" + "github.com/codeGROOVE-dev/retry" "github.com/energye/systray" "github.com/gen2brain/beeep" @@ -38,8 +40,9 @@ const ( stalePRThreshold = 90 * 24 * time.Hour maxPRsToProcess = 200 // Limit for performance - // Update intervals. - updateInterval = 5 * time.Minute // Reduced frequency to avoid rate limits + // Update interval settings. + minUpdateInterval = 10 * time.Second + defaultUpdateInterval = 1 * time.Minute // Retry settings for external API calls - exponential backoff with jitter up to 2 minutes. maxRetryDelay = 2 * time.Minute @@ -58,6 +61,32 @@ type PR struct { NeedsReview bool } +// MenuState represents the current state of menu items for comparison. +type MenuState struct { + IncomingItems []MenuItemState `json:"incoming"` + OutgoingItems []MenuItemState `json:"outgoing"` + HideStale bool `json:"hide_stale"` +} + +// MenuItemState represents a single menu item's display state. +type MenuItemState struct { + URL string `json:"url"` + Title string `json:"title"` + Repository string `json:"repository"` + ActionReason string `json:"action_reason"` + Number int `json:"number"` + NeedsReview bool `json:"needs_review"` + IsBlocked bool `json:"is_blocked"` +} + +// TurnResult represents a Turn API result to be applied later. +type TurnResult struct { + URL string + ActionReason string + NeedsReview bool + IsOwner bool +} + // App holds the application state. type App struct { lastSuccessfulFetch time.Time @@ -65,30 +94,41 @@ type App struct { currentUser *github.User previousBlockedPRs map[string]bool client *github.Client + lastMenuState *MenuState targetUser string cacheDir string - outgoing []PR incoming []PR - lastMenuHashInt uint64 + outgoing []PR + pendingTurnResults []TurnResult consecutiveFailures int + updateInterval time.Duration mu sync.RWMutex initialLoadComplete bool menuInitialized bool noCache bool hideStaleIncoming bool + loadingTurnData bool } func main() { // Parse command line flags var targetUser string var noCache bool + var updateInterval time.Duration flag.StringVar(&targetUser, "user", "", "GitHub user to query PRs for (defaults to authenticated user)") flag.BoolVar(&noCache, "no-cache", false, "Bypass cache for debugging") + flag.DurationVar(&updateInterval, "interval", defaultUpdateInterval, "Update interval (e.g. 30s, 1m, 5m)") flag.Parse() + // Validate update interval + if updateInterval < minUpdateInterval { + log.Printf("Update interval %v too short, using minimum of %v", updateInterval, minUpdateInterval) + updateInterval = minUpdateInterval + } + log.SetFlags(log.LstdFlags | log.Lshortfile) log.Printf("Starting GitHub PR Monitor (version=%s, commit=%s, date=%s)", version, commit, date) - log.Printf("Retry configuration: max_retries=%d, max_delay=%v", maxRetries, maxRetryDelay) + log.Printf("Configuration: update_interval=%v, max_retries=%d, max_delay=%v", updateInterval, maxRetries, maxRetryDelay) ctx := context.Background() @@ -108,6 +148,8 @@ func main() { previousBlockedPRs: make(map[string]bool), targetUser: targetUser, noCache: noCache, + updateInterval: updateInterval, + pendingTurnResults: make([]TurnResult, 0), } log.Println("Initializing GitHub clients...") @@ -218,8 +260,9 @@ func (app *App) updateLoop(ctx context.Context) { } }() - ticker := time.NewTicker(updateInterval) + ticker := time.NewTicker(app.updateInterval) defer ticker.Stop() + log.Printf("[UPDATE] Update loop started with interval: %v", app.updateInterval) // Initial update with wait for Turn data app.updatePRsWithWait(ctx) @@ -299,6 +342,7 @@ 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 @@ -308,9 +352,11 @@ func (app *App) updatePRs(ctx context.Context) { // Use a single lock for all operations on previousBlockedPRs and initialLoadComplete app.mu.Lock() oldBlockedPRs := app.previousBlockedPRs - initialLoad := app.initialLoadComplete + 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 @@ -323,12 +369,16 @@ func (app *App) updatePRs(ctx context.Context) { } // Send notification and play sound if PR wasn't blocked before // (only after initial load to avoid startup noise) - if initialLoad && !oldBlockedPRs[incoming[i].URL] { + 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]) } } } @@ -341,12 +391,16 @@ func (app *App) updatePRs(ctx context.Context) { } // Send notification and play sound if PR wasn't blocked before // (only after initial load to avoid startup noise) - if initialLoad && !oldBlockedPRs[outgoing[i].URL] { + 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]) } } } @@ -365,56 +419,113 @@ func (app *App) updatePRs(ctx context.Context) { app.updateMenuIfChanged(ctx) } -// updateMenuIfChanged only rebuilds the menu if the PR data has actually changed. -func (app *App) updateMenuIfChanged(ctx context.Context) { - app.mu.RLock() - // Calculate hash including blocking status - use efficient integer hash - var incomingBlocked, outgoingBlocked int - for i := range app.incoming { - if app.incoming[i].NeedsReview { - incomingBlocked++ +// buildCurrentMenuState creates a MenuState representing the current menu items. +func (app *App) buildCurrentMenuState() *MenuState { + // Apply the same filtering as the menu display (stale PR filtering) + var filteredIncoming, filteredOutgoing []PR + + now := time.Now() + staleThreshold := now.Add(-stalePRThreshold) + + for _, pr := range app.incoming { + if !app.hideStaleIncoming || pr.UpdatedAt.After(staleThreshold) { + filteredIncoming = append(filteredIncoming, pr) } } - for i := range app.outgoing { - if app.outgoing[i].IsBlocked { - outgoingBlocked++ + + for _, pr := range app.outgoing { + if !app.hideStaleIncoming || pr.UpdatedAt.After(staleThreshold) { + filteredOutgoing = append(filteredOutgoing, pr) } } - // Build hash as integers to avoid string allocation - const ( - hashPartsCount = 5 - hideStaleIndex = 4 - bitsPerHashPart = 8 - ) - hashParts := [hashPartsCount]int{ - len(app.incoming), - len(app.outgoing), - incomingBlocked, - outgoingBlocked, - 0, // hideStaleIncoming + // Sort PRs the same way the menu does + incomingSorted := sortPRsBlockedFirst(filteredIncoming) + outgoingSorted := sortPRsBlockedFirst(filteredOutgoing) + + // Build menu item states + incomingItems := make([]MenuItemState, len(incomingSorted)) + for i, pr := range incomingSorted { + incomingItems[i] = MenuItemState{ + URL: pr.URL, + Title: pr.Title, + Repository: pr.Repository, + Number: pr.Number, + NeedsReview: pr.NeedsReview, + IsBlocked: false, // incoming PRs don't use IsBlocked + ActionReason: pr.ActionReason, + } + } + + outgoingItems := make([]MenuItemState, len(outgoingSorted)) + for i, pr := range outgoingSorted { + outgoingItems[i] = MenuItemState{ + URL: pr.URL, + Title: pr.Title, + Repository: pr.Repository, + Number: pr.Number, + NeedsReview: pr.NeedsReview, + IsBlocked: pr.IsBlocked, + ActionReason: pr.ActionReason, + } } - if app.hideStaleIncoming { - hashParts[hideStaleIndex] = 1 + + return &MenuState{ + IncomingItems: incomingItems, + OutgoingItems: outgoingItems, + HideStale: app.hideStaleIncoming, } +} - // Simple hash function - combine values - var currentHashInt uint64 - for i, part := range hashParts { - // Safe conversion since we control the values - if part >= 0 { - currentHashInt ^= uint64(part) << (i * bitsPerHashPart) +// updateMenuIfChanged only rebuilds the menu if the PR data has actually changed. +func (app *App) updateMenuIfChanged(ctx context.Context) { + app.mu.RLock() + // Skip menu updates while Turn data is still loading to avoid excessive rebuilds + if app.loadingTurnData { + // But still save the current state for future comparisons + currentMenuState := app.buildCurrentMenuState() + if app.lastMenuState == nil { + log.Print("[MENU] Skipping menu update - Turn data still loading, but saving state for future comparison") + app.mu.RUnlock() + app.mu.Lock() + app.lastMenuState = currentMenuState + app.mu.Unlock() + } else { + app.mu.RUnlock() + 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 currentHashInt == app.lastMenuHashInt { - log.Printf("[MENU] Menu hash unchanged (%d), skipping update", currentHashInt) - return + 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)) } - log.Printf("[MENU] Menu hash changed from %d to %d, rebuilding entire menu", app.lastMenuHashInt, currentHashInt) - app.lastMenuHashInt = currentHashInt + 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) } @@ -461,6 +572,7 @@ func (app *App) updatePRsWithWait(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 @@ -470,9 +582,11 @@ func (app *App) updatePRsWithWait(ctx context.Context) { // Use a single lock for all operations on previousBlockedPRs and initialLoadComplete app.mu.Lock() oldBlockedPRs := app.previousBlockedPRs - initialLoad := app.initialLoadComplete + 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 @@ -485,12 +599,16 @@ func (app *App) updatePRsWithWait(ctx context.Context) { } // Send notification and play sound if PR wasn't blocked before // (only after initial load to avoid startup noise) - if initialLoad && !oldBlockedPRs[incoming[i].URL] { + 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]) } } } @@ -503,12 +621,16 @@ func (app *App) updatePRsWithWait(ctx context.Context) { } // Send notification and play sound if PR wasn't blocked before // (only after initial load to avoid startup noise) - if initialLoad && !oldBlockedPRs[outgoing[i].URL] { + 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]) } } } diff --git a/main_test.go b/main_test.go index 97534f3..9f9b12e 100644 --- a/main_test.go +++ b/main_test.go @@ -37,38 +37,3 @@ func TestIsStale(t *testing.T) { }) } } - -// TestEscapeAppleScriptString tests were removed as the function was replaced -// with validateAndEscapePathForAppleScript which is not exported -func TestEscapeAppleScriptString(t *testing.T) { - t.Skip("escapeAppleScriptString was replaced with validateAndEscapePathForAppleScript") - tests := []struct { - name string - input string - expected string - }{ - { - name: "simple string", - input: "hello world", - expected: "hello world", - }, - { - name: "string with quotes", - input: `hello "world"`, - expected: `hello \"world\"`, - }, - { - name: "string with backslashes", - input: `hello\world`, - expected: `hello\\world`, - }, - { - name: "complex string", - input: `path\to\"file"`, - expected: `path\\to\\\"file\"`, - }, - } - - // Tests removed - function was replaced - _ = tests -} diff --git a/sound.go b/sound.go index 998669a..6462603 100644 --- a/sound.go +++ b/sound.go @@ -53,6 +53,7 @@ func (app *App) initSoundCache() { // playSound plays a cached sound file using platform-specific commands. func (app *App) playSound(ctx context.Context, soundType string) { + log.Printf("[SOUND] Playing %s sound", soundType) // Ensure sounds are cached app.initSoundCache() diff --git a/ui.go b/ui.go index 3338372..1e7d2e5 100644 --- a/ui.go +++ b/ui.go @@ -300,7 +300,10 @@ func (app *App) addStaticMenuItems(ctx context.Context) { } log.Printf("Hide stale PRs: %v", app.hideStaleIncoming) // Force menu rebuild since hideStaleIncoming changed - app.lastMenuHashInt = 0 + app.mu.Lock() + log.Print("[MENU] *** CLEARING menu state due to hide stale toggle ***") + app.lastMenuState = nil + app.mu.Unlock() app.rebuildMenu(ctx) })