Skip to content

Commit 47089e4

Browse files
tstrombergtstromberg
committed
sort blocking PRs first
1 parent 56300e9 commit 47089e4

File tree

6 files changed

+518
-368
lines changed

6 files changed

+518
-368
lines changed

‎github.go‎

Lines changed: 217 additions & 25 deletions
Original file line numberDiff line numberDiff line change
@@ -270,16 +270,218 @@ func (app *App) updatePRData(url string, needsReview bool, isOwner bool, actionR
270270
return nil
271271
}
272272

273+
// fetchPRsWithWait fetches PRs and waits for Turn data to complete.
274+
func (app *App) fetchPRsWithWait(ctx context.Context) (incoming []PR, outgoing []PR, err error) {
275+
// Use targetUser if specified, otherwise use authenticated user
276+
user := app.currentUser.GetLogin()
277+
if app.targetUser != "" {
278+
user = app.targetUser
279+
}
280+
281+
// Single query to get all PRs involving the user
282+
query := fmt.Sprintf("is:open is:pr involves:%s archived:false", user)
283+
284+
const perPage = 100
285+
opts := &github.SearchOptions{
286+
ListOptions: github.ListOptions{PerPage: perPage},
287+
Sort: "updated",
288+
Order: "desc",
289+
}
290+
291+
log.Printf("Searching for PRs with query: %s", query)
292+
searchStart := time.Now()
293+
294+
var result *github.IssuesSearchResult
295+
var resp *github.Response
296+
err = retry.Do(func() error {
297+
// Create timeout context for GitHub API call
298+
githubCtx, cancel := context.WithTimeout(ctx, 30*time.Second)
299+
defer cancel()
300+
301+
var retryErr error
302+
result, resp, retryErr = app.client.Search.Issues(githubCtx, query, opts)
303+
if retryErr != nil {
304+
// Enhanced error handling with specific cases
305+
if resp != nil {
306+
const (
307+
httpStatusUnauthorized = 401
308+
httpStatusForbidden = 403
309+
httpStatusUnprocessable = 422
310+
)
311+
switch resp.StatusCode {
312+
case httpStatusForbidden:
313+
if resp.Header.Get("X-Ratelimit-Remaining") == "0" {
314+
resetTime := resp.Header.Get("X-Ratelimit-Reset")
315+
log.Printf("GitHub API rate limited, reset at: %s (will retry)", resetTime)
316+
return retryErr // Retry on rate limit
317+
}
318+
log.Print("GitHub API access forbidden (check token permissions)")
319+
return retry.Unrecoverable(fmt.Errorf("github API access forbidden: %w", retryErr))
320+
case httpStatusUnauthorized:
321+
log.Print("GitHub API authentication failed (check token)")
322+
return retry.Unrecoverable(fmt.Errorf("github API authentication failed: %w", retryErr))
323+
case httpStatusUnprocessable:
324+
log.Printf("GitHub API query invalid: %s", query)
325+
return retry.Unrecoverable(fmt.Errorf("github API query invalid: %w", retryErr))
326+
default:
327+
log.Printf("GitHub API error (status %d): %v (will retry)", resp.StatusCode, retryErr)
328+
}
329+
} else {
330+
// Likely network error - retry these
331+
log.Printf("GitHub API network error: %v (will retry)", retryErr)
332+
}
333+
return retryErr
334+
}
335+
return nil
336+
},
337+
retry.Attempts(maxRetries),
338+
retry.DelayType(retry.BackOffDelay),
339+
retry.MaxDelay(maxRetryDelay),
340+
retry.OnRetry(func(n uint, err error) {
341+
log.Printf("GitHub Search.Issues retry %d/%d: %v", n+1, maxRetries, err)
342+
}),
343+
retry.Context(ctx),
344+
)
345+
if err != nil {
346+
return nil, nil, fmt.Errorf("search PRs after %d retries: %w", maxRetries, err)
347+
}
348+
349+
log.Printf("GitHub search completed in %v, found %d PRs", time.Since(searchStart), len(result.Issues))
350+
351+
// Limit PRs for performance
352+
if len(result.Issues) > maxPRsToProcess {
353+
log.Printf("Limiting to %d PRs for performance (total: %d)", maxPRsToProcess, len(result.Issues))
354+
result.Issues = result.Issues[:maxPRsToProcess]
355+
}
356+
357+
// Process GitHub results
358+
for _, issue := range result.Issues {
359+
if !issue.IsPullRequest() {
360+
continue
361+
}
362+
repo := strings.TrimPrefix(issue.GetRepositoryURL(), "https://api.github.com/repos/")
363+
364+
pr := PR{
365+
Title: issue.GetTitle(),
366+
URL: issue.GetHTMLURL(),
367+
Repository: repo,
368+
Number: issue.GetNumber(),
369+
UpdatedAt: issue.GetUpdatedAt().Time,
370+
}
371+
372+
// Categorize as incoming or outgoing
373+
if issue.GetUser().GetLogin() == user {
374+
outgoing = append(outgoing, pr)
375+
} else {
376+
incoming = append(incoming, pr)
377+
}
378+
}
379+
380+
log.Printf("[GITHUB] Found %d incoming, %d outgoing PRs from GitHub", len(incoming), len(outgoing))
381+
382+
// Now fetch Turn data synchronously
383+
log.Println("[TURN] Fetching Turn API data synchronously before building menu...")
384+
app.fetchTurnDataSync(ctx, result.Issues, user, &incoming, &outgoing)
385+
386+
return incoming, outgoing, nil
387+
}
388+
389+
// fetchTurnDataSync fetches Turn API data synchronously and updates PRs directly.
390+
func (app *App) fetchTurnDataSync(ctx context.Context, issues []*github.Issue, user string, incoming *[]PR, outgoing *[]PR) {
391+
turnStart := time.Now()
392+
type prResult struct {
393+
err error
394+
turnData *turn.CheckResponse
395+
url string
396+
isOwner bool
397+
}
398+
399+
// Create a channel for results
400+
results := make(chan prResult, len(issues))
401+
402+
// Use a WaitGroup to track goroutines
403+
var wg sync.WaitGroup
404+
405+
// Process PRs in parallel
406+
for _, issue := range issues {
407+
if !issue.IsPullRequest() {
408+
continue
409+
}
410+
411+
wg.Add(1)
412+
go func(issue *github.Issue) {
413+
defer wg.Done()
414+
415+
url := issue.GetHTMLURL()
416+
updatedAt := issue.GetUpdatedAt().Time
417+
418+
// Call turnData - it now has proper exponential backoff with jitter
419+
turnData, err := app.turnData(ctx, url, updatedAt)
420+
421+
results <- prResult{
422+
url: issue.GetHTMLURL(),
423+
turnData: turnData,
424+
err: err,
425+
isOwner: issue.GetUser().GetLogin() == user,
426+
}
427+
}(issue)
428+
}
429+
430+
// Close the results channel when all goroutines are done
431+
go func() {
432+
wg.Wait()
433+
close(results)
434+
}()
435+
436+
// Collect results and update PRs directly
437+
turnSuccesses := 0
438+
turnFailures := 0
439+
440+
for result := range results {
441+
if result.err == nil && result.turnData != nil && result.turnData.PRState.UnblockAction != nil {
442+
turnSuccesses++
443+
444+
// Check if user needs to review and get action reason
445+
needsReview := false
446+
actionReason := ""
447+
if action, exists := result.turnData.PRState.UnblockAction[user]; exists {
448+
needsReview = true
449+
actionReason = action.Reason
450+
log.Printf("[TURN] UnblockAction for %s: Reason=%q, Kind=%q", result.url, action.Reason, action.Kind)
451+
}
452+
453+
// Update the PR in the slices directly
454+
if result.isOwner {
455+
for i := range *outgoing {
456+
if (*outgoing)[i].URL == result.url {
457+
(*outgoing)[i].NeedsReview = needsReview
458+
(*outgoing)[i].IsBlocked = needsReview
459+
(*outgoing)[i].ActionReason = actionReason
460+
break
461+
}
462+
}
463+
} else {
464+
for i := range *incoming {
465+
if (*incoming)[i].URL == result.url {
466+
(*incoming)[i].NeedsReview = needsReview
467+
(*incoming)[i].ActionReason = actionReason
468+
break
469+
}
470+
}
471+
}
472+
} else if result.err != nil {
473+
turnFailures++
474+
}
475+
}
476+
477+
log.Printf("[TURN] Turn API queries completed in %v (%d/%d succeeded)",
478+
time.Since(turnStart), turnSuccesses, turnSuccesses+turnFailures)
479+
}
480+
273481
// fetchTurnDataAsync fetches Turn API data in the background and updates PRs as results arrive.
274482
func (app *App) fetchTurnDataAsync(ctx context.Context, issues []*github.Issue, user string) {
275-
// Set loading state
276-
app.mu.Lock()
277-
app.turnDataLoading = true
278-
app.mu.Unlock()
279-
280-
// Update section headers to show loading state
281-
log.Print("[TURN] Starting Turn API queries, updating section headers to show loading state")
282-
app.updateSectionHeaders()
483+
// Log start of Turn API queries
484+
log.Print("[TURN] Starting Turn API queries in background")
283485

284486
turnStart := time.Now()
285487
type prResult struct {
@@ -359,13 +561,10 @@ func (app *App) fetchTurnDataAsync(ctx context.Context, issues []*github.Issue,
359561
updatesApplied++
360562
updateBatch++
361563
log.Printf("[TURN] Turn data received for %s (needsReview=%v, actionReason=%q)", result.url, needsReview, actionReason)
362-
// Update the specific menu item immediately
363-
app.updatePRMenuItem(*pr)
364564

365-
// Periodically update section headers and tray title
565+
// Periodically update tray title
366566
if updateBatch >= batchSize || time.Since(lastUpdateTime) >= minUpdateInterval {
367-
log.Printf("[TURN] Batch update threshold reached (%d updates), updating headers and title", updateBatch)
368-
app.updateSectionHeaders()
567+
log.Printf("[TURN] Batch update threshold reached (%d updates), updating title", updateBatch)
369568
app.setTrayTitle()
370569
updateBatch = 0
371570
lastUpdateTime = time.Now()
@@ -376,19 +575,12 @@ func (app *App) fetchTurnDataAsync(ctx context.Context, issues []*github.Issue,
376575
}
377576
}
378577

379-
// Clear loading state
380-
app.mu.Lock()
381-
app.turnDataLoading = false
382-
app.turnDataLoaded = true
383-
app.mu.Unlock()
384-
385578
log.Printf("[TURN] Turn API queries completed in %v (%d/%d succeeded, %d PRs updated)",
386579
time.Since(turnStart), turnSuccesses, turnSuccesses+turnFailures, updatesApplied)
387580

388-
// Update section headers with final counts
389-
log.Print("[TURN] Updating section headers and tray title with final counts")
390-
app.updateSectionHeaders()
391-
392-
// Update tray title
393-
app.setTrayTitle()
581+
// Rebuild menu with final Turn data if menu is already initialized
582+
if app.menuInitialized {
583+
log.Print("[TURN] Turn data loaded, rebuilding menu")
584+
app.rebuildMenu(ctx)
585+
}
394586
}

‎loginitem_darwin.go‎

Lines changed: 33 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -16,14 +16,22 @@ import (
1616
"github.com/energye/systray"
1717
)
1818

19-
// escapeAppleScriptString safely escapes a string for use in AppleScript.
20-
// This prevents injection attacks by escaping quotes and backslashes.
21-
func escapeAppleScriptString(s string) string {
22-
// Escape backslashes first
23-
s = strings.ReplaceAll(s, `\`, `\\`)
24-
// Then escape quotes
25-
s = strings.ReplaceAll(s, `"`, `\"`)
26-
return s
19+
// validateAndEscapePathForAppleScript validates and escapes a path for safe use in AppleScript.
20+
// Returns empty string if path contains invalid characters.
21+
func validateAndEscapePathForAppleScript(path string) string {
22+
// Validate path contains only safe characters
23+
for _, r := range path {
24+
if (r < 'a' || r > 'z') && (r < 'A' || r > 'Z') &&
25+
(r < '0' || r > '9') && r != ' ' && r != '.' &&
26+
r != '/' && r != '-' && r != '_' {
27+
log.Printf("Path contains invalid character for AppleScript: %q in path %s", r, path)
28+
return ""
29+
}
30+
}
31+
// Escape backslashes first then quotes
32+
path = strings.ReplaceAll(path, `\`, `\\`)
33+
path = strings.ReplaceAll(path, `"`, `\"`)
34+
return path
2735
}
2836

2937
// isLoginItem checks if the app is set to start at login.
@@ -35,8 +43,12 @@ func isLoginItem(ctx context.Context) bool {
3543
}
3644

3745
// Use osascript to check login items
38-
escapedPath := escapeAppleScriptString(appPath)
39-
// We use %s here because the string is already escaped by escapeAppleScriptString
46+
escapedPath := validateAndEscapePathForAppleScript(appPath)
47+
if escapedPath == "" {
48+
log.Printf("Invalid app path for AppleScript: %s", appPath)
49+
return false
50+
}
51+
// We use %s here because the string is already validated and escaped
4052
//nolint:gocritic // already escaped
4153
script := fmt.Sprintf(
4254
`tell application "System Events" to get the name of every login item where path is "%s"`,
@@ -62,8 +74,11 @@ func setLoginItem(ctx context.Context, enable bool) error {
6274

6375
if enable {
6476
// Add to login items
65-
escapedPath := escapeAppleScriptString(appPath)
66-
// We use %s here because the string is already escaped by escapeAppleScriptString
77+
escapedPath := validateAndEscapePathForAppleScript(appPath)
78+
if escapedPath == "" {
79+
return fmt.Errorf("invalid app path for AppleScript: %s", appPath)
80+
}
81+
// We use %s here because the string is already validated and escaped
6782
//nolint:gocritic // already escaped
6883
script := fmt.Sprintf(
6984
`tell application "System Events" to make login item at end with properties {path:"%s", hidden:false}`,
@@ -78,8 +93,11 @@ func setLoginItem(ctx context.Context, enable bool) error {
7893
// Remove from login items
7994
appName := filepath.Base(appPath)
8095
appName = strings.TrimSuffix(appName, ".app")
81-
escapedName := escapeAppleScriptString(appName)
82-
// We use %s here because the string is already escaped by escapeAppleScriptString
96+
escapedName := validateAndEscapePathForAppleScript(appName)
97+
if escapedName == "" {
98+
return fmt.Errorf("invalid app name for AppleScript: %s", appName)
99+
}
100+
// We use %s here because the string is already validated and escaped
83101
script := fmt.Sprintf(`tell application "System Events" to delete login item "%s"`, escapedName) //nolint:gocritic // already escaped
84102
log.Printf("Executing command: osascript -e %q", script)
85103
cmd := exec.CommandContext(ctx, "osascript", "-e", script)
@@ -142,15 +160,14 @@ func getAppPath() (string, error) {
142160
}
143161

144162
// addLoginItemUI adds the login item menu option (macOS only).
145-
func addLoginItemUI(ctx context.Context, app *App) {
163+
func addLoginItemUI(ctx context.Context, _ *App) {
146164
// Only show login item menu if running from an app bundle
147165
if !isRunningFromAppBundle() {
148166
log.Println("Hiding 'Start at Login' menu item - not running from app bundle")
149167
return
150168
}
151169

152170
loginItem := systray.AddMenuItem("Start at Login", "Automatically start when you log in")
153-
app.menuItems = append(app.menuItems, loginItem)
154171

155172
// Set initial state
156173
if isLoginItem(ctx) {

0 commit comments

Comments
 (0)