Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
93 changes: 66 additions & 27 deletions cmd/goose/cache.go
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,8 @@ type cacheEntry struct {

// turnData fetches Turn API data with caching.
func (app *App) turnData(ctx context.Context, url string, updatedAt time.Time) (*turn.CheckResponse, bool, error) {
prAge := time.Since(updatedAt)
hasRunningTests := false
// Validate URL before processing
if err := validateURL(url); err != nil {
return nil, false, fmt.Errorf("invalid URL: %w", err)
Expand Down Expand Up @@ -55,15 +57,26 @@ func (app *App) turnData(ctx context.Context, url string, updatedAt time.Time) (
}
} else if time.Since(entry.CachedAt) < cacheTTL && entry.UpdatedAt.Equal(updatedAt) {
// Check if cache is still valid (10 day TTL, but PR UpdatedAt is primary check)
slog.Debug("[CACHE] Cache hit",
"url", url,
"cached_at", entry.CachedAt.Format(time.RFC3339),
"cache_age", time.Since(entry.CachedAt).Round(time.Second),
"pr_updated_at", entry.UpdatedAt.Format(time.RFC3339))
if app.healthMonitor != nil {
app.healthMonitor.recordCacheAccess(true)
// But invalidate cache for PRs with running tests if they're fresh (< 90 minutes old)
if entry.Data != nil && entry.Data.PullRequest.TestState == "running" && prAge < runningTestsCacheBypass {
hasRunningTests = true
slog.Debug("[CACHE] Cache invalidated - PR has running tests and is fresh",
"url", url,
"test_state", entry.Data.PullRequest.TestState,
"pr_age", prAge.Round(time.Minute),
"cached_at", entry.CachedAt.Format(time.RFC3339))
// Don't return cached data - fall through to fetch fresh data with current time
} else {
slog.Debug("[CACHE] Cache hit",
"url", url,
"cached_at", entry.CachedAt.Format(time.RFC3339),
"cache_age", time.Since(entry.CachedAt).Round(time.Second),
"pr_updated_at", entry.UpdatedAt.Format(time.RFC3339))
if app.healthMonitor != nil {
app.healthMonitor.recordCacheAccess(true)
}
return entry.Data, true, nil
}
return entry.Data, true, nil
} else {
// Log why cache was invalid
if !entry.UpdatedAt.Equal(updatedAt) {
Expand Down Expand Up @@ -103,12 +116,22 @@ func (app *App) turnData(ctx context.Context, url string, updatedAt time.Time) (
turnCtx, cancel := context.WithTimeout(ctx, turnAPITimeout)
defer cancel()

// For PRs with running tests, send current time to bypass Turn server cache
timestampToSend := updatedAt
if hasRunningTests {
timestampToSend = time.Now()
slog.Debug("[TURN] Using current timestamp for PR with running tests to bypass Turn server cache",
"url", url,
"pr_updated_at", updatedAt.Format(time.RFC3339),
"timestamp_sent", timestampToSend.Format(time.RFC3339))
}

var retryErr error
slog.Debug("[TURN] Making API call",
"url", url,
"user", app.currentUser.GetLogin(),
"pr_updated_at", updatedAt.Format(time.RFC3339))
data, retryErr = app.turnClient.Check(turnCtx, url, app.currentUser.GetLogin(), updatedAt)
"pr_updated_at", timestampToSend.Format(time.RFC3339))
data, retryErr = app.turnClient.Check(turnCtx, url, app.currentUser.GetLogin(), timestampToSend)
if retryErr != nil {
slog.Warn("Turn API error (will retry)", "error", retryErr)
return retryErr
Expand Down Expand Up @@ -137,26 +160,42 @@ func (app *App) turnData(ctx context.Context, url string, updatedAt time.Time) (
}

// Save to cache (don't fail if caching fails) - skip if --no-cache is set
// Also skip caching if tests are running and PR is fresh (updated in last 90 minutes)
if !app.noCache {
entry := cacheEntry{
Data: data,
CachedAt: time.Now(),
UpdatedAt: updatedAt,
shouldCache := true
prAge := time.Since(updatedAt)

// Don't cache PRs with running tests unless they're older than 90 minutes
if data != nil && data.PullRequest.TestState == "running" && prAge < runningTestsCacheBypass {
shouldCache = false
slog.Debug("[CACHE] Skipping cache for PR with running tests",
"url", url,
"test_state", data.PullRequest.TestState,
"pr_age", prAge.Round(time.Minute),
"pending_checks", len(data.PullRequest.CheckSummary.PendingStatuses))
}
if cacheData, marshalErr := json.Marshal(entry); marshalErr != nil {
slog.Error("Failed to marshal cache data", "url", url, "error", marshalErr)
} else {
// Ensure cache directory exists with secure permissions
if dirErr := os.MkdirAll(filepath.Dir(cacheFile), 0o700); dirErr != nil {
slog.Error("Failed to create cache directory", "error", dirErr)
} else if writeErr := os.WriteFile(cacheFile, cacheData, 0o600); writeErr != nil {
slog.Error("Failed to write cache file", "error", writeErr)

if shouldCache {
entry := cacheEntry{
Data: data,
CachedAt: time.Now(),
UpdatedAt: updatedAt,
}
if cacheData, marshalErr := json.Marshal(entry); marshalErr != nil {
slog.Error("Failed to marshal cache data", "url", url, "error", marshalErr)
} else {
slog.Debug("[CACHE] Saved to cache",
"url", url,
"cached_at", entry.CachedAt.Format(time.RFC3339),
"pr_updated_at", entry.UpdatedAt.Format(time.RFC3339),
"cache_file", filepath.Base(cacheFile))
// Ensure cache directory exists with secure permissions
if dirErr := os.MkdirAll(filepath.Dir(cacheFile), 0o700); dirErr != nil {
slog.Error("Failed to create cache directory", "error", dirErr)
} else if writeErr := os.WriteFile(cacheFile, cacheData, 0o600); writeErr != nil {
slog.Error("Failed to write cache file", "error", writeErr)
} else {
slog.Debug("[CACHE] Saved to cache",
"url", url,
"cached_at", entry.CachedAt.Format(time.RFC3339),
"pr_updated_at", entry.UpdatedAt.Format(time.RFC3339),
"cache_file", filepath.Base(cacheFile))
}
}
}
}
Expand Down
3 changes: 3 additions & 0 deletions cmd/goose/github.go
Original file line number Diff line number Diff line change
Expand Up @@ -481,6 +481,7 @@ func (app *App) fetchTurnDataSync(ctx context.Context, issues []*github.Issue, u
isBlocked := false
actionReason := ""
actionKind := ""
testState := result.turnData.PullRequest.TestState
if action, exists := result.turnData.Analysis.NextAction[user]; exists {
needsReview = true
isBlocked = action.Critical // Only critical actions are blocking
Expand All @@ -502,6 +503,7 @@ func (app *App) fetchTurnDataSync(ctx context.Context, issues []*github.Issue, u
(*outgoing)[i].IsBlocked = isBlocked
(*outgoing)[i].ActionReason = actionReason
(*outgoing)[i].ActionKind = actionKind
(*outgoing)[i].TestState = testState
break
}
} else {
Expand All @@ -512,6 +514,7 @@ func (app *App) fetchTurnDataSync(ctx context.Context, issues []*github.Issue, u
(*incoming)[i].NeedsReview = needsReview
(*incoming)[i].ActionReason = actionReason
(*incoming)[i].ActionKind = actionKind
(*incoming)[i].TestState = testState
break
}
}
Expand Down
14 changes: 7 additions & 7 deletions cmd/goose/icons.go
Original file line number Diff line number Diff line change
Expand Up @@ -7,10 +7,10 @@ import (
)

// Icon variables are defined in platform-specific files:
// - icons_windows.go: uses .ico files
// - icons_unix.go: uses .png files
// - icons_windows.go: uses .ico files.
// - icons_unix.go: uses .png files.

// IconType represents different icon states
// IconType represents different icon states.
type IconType int

const (
Expand All @@ -22,7 +22,7 @@ const (
IconLock // Authentication error
)

// getIcon returns the icon bytes for the given type
// getIcon returns the icon bytes for the given type.
func getIcon(iconType IconType) []byte {
switch iconType {
case IconGoose:
Expand All @@ -43,7 +43,7 @@ func getIcon(iconType IconType) []byte {
}
}

// loadIconFromFile loads an icon from the filesystem (fallback if embed fails)
// loadIconFromFile loads an icon from the filesystem (fallback if embed fails).
func loadIconFromFile(filename string) []byte {
iconPath := filepath.Join("icons", filename)
data, err := os.ReadFile(iconPath)
Expand All @@ -54,7 +54,7 @@ func loadIconFromFile(filename string) []byte {
return data
}

// setTrayIcon updates the system tray icon based on PR counts
// setTrayIcon updates the system tray icon based on PR counts.
func (app *App) setTrayIcon(iconType IconType) {
iconBytes := getIcon(iconType)
if iconBytes == nil || len(iconBytes) == 0 {
Expand All @@ -64,4 +64,4 @@ func (app *App) setTrayIcon(iconType IconType) {

app.systrayInterface.SetIcon(iconBytes)
slog.Debug("[TRAY] Setting icon", "type", iconType)
}
}
2 changes: 1 addition & 1 deletion cmd/goose/icons_unix.go
Original file line number Diff line number Diff line change
Expand Up @@ -21,4 +21,4 @@ var iconSmiling []byte
var iconLock []byte

//go:embed icons/warning.png
var iconWarning []byte
var iconWarning []byte
68 changes: 35 additions & 33 deletions cmd/goose/main.go
Original file line number Diff line number Diff line change
Expand Up @@ -34,6 +34,7 @@
cacheTTL = 10 * 24 * time.Hour // 10 days - rely mostly on PR UpdatedAt
cacheCleanupInterval = 15 * 24 * time.Hour // 15 days - cleanup older than cache TTL
stalePRThreshold = 90 * 24 * time.Hour
runningTestsCacheBypass = 90 * time.Minute // Don't cache PRs with running tests if fresher than this
maxPRsToProcess = 200
minUpdateInterval = 10 * time.Second
defaultUpdateInterval = 1 * time.Minute
Expand All @@ -46,7 +47,7 @@
turnAPITimeout = 10 * time.Second
maxConcurrentTurnAPICalls = 20
defaultMaxBrowserOpensDay = 20
startupGracePeriod = 1 * time.Minute // Don't play sounds or auto-open for first minute
startupGracePeriod = 1 * time.Minute // Don't play sounds or auto-open for first minute
)

// PR represents a pull request with metadata.
Expand All @@ -60,6 +61,7 @@
Author string // GitHub username of the PR author
ActionReason string
ActionKind string // The kind of action expected (review, merge, fix_tests, etc.)
TestState string // Test state from Turn API: "running", "passing", "failing", etc.
Number int
IsDraft bool
IsBlocked bool
Expand All @@ -67,40 +69,40 @@
}

// App holds the application state.
type App struct {

Check failure on line 72 in cmd/goose/main.go

View workflow job for this annotation

GitHub Actions / golangci-lint

fieldalignment: struct with 368 pointer bytes could be 296 (govet)
lastSearchAttempt time.Time
lastSuccessfulFetch time.Time
startTime time.Time
systrayInterface SystrayInterface
browserRateLimiter *BrowserRateLimiter
blockedPRTimes map[string]time.Time
currentUser *github.User
stateManager *PRStateManager
client *github.Client
hiddenOrgs map[string]bool
seenOrgs map[string]bool
turnClient *turn.Client
previousBlockedPRs map[string]bool
authError string
lastFetchError string
cacheDir string
targetUser string
lastMenuTitles []string
outgoing []PR
incoming []PR
updateInterval time.Duration
consecutiveFailures int
mu sync.RWMutex
menuMutex sync.Mutex // Mutex to prevent concurrent menu rebuilds
enableAutoBrowser bool
hideStaleIncoming bool
lastSearchAttempt time.Time
lastSuccessfulFetch time.Time
startTime time.Time
systrayInterface SystrayInterface
browserRateLimiter *BrowserRateLimiter
blockedPRTimes map[string]time.Time
currentUser *github.User
stateManager *PRStateManager
client *github.Client
hiddenOrgs map[string]bool
seenOrgs map[string]bool
turnClient *turn.Client
previousBlockedPRs map[string]bool
authError string
lastFetchError string
cacheDir string
targetUser string
lastMenuTitles []string
outgoing []PR
incoming []PR
updateInterval time.Duration
consecutiveFailures int
mu sync.RWMutex
menuMutex sync.Mutex // Mutex to prevent concurrent menu rebuilds
enableAutoBrowser bool
hideStaleIncoming bool
hasPerformedInitialDiscovery bool // Track if we've done the first poll to distinguish from real state changes
noCache bool
enableAudioCues bool
initialLoadComplete bool
menuInitialized bool
healthMonitor *healthMonitor
githubCircuit *circuitBreaker
noCache bool
enableAudioCues bool
initialLoadComplete bool
menuInitialized bool
healthMonitor *healthMonitor
githubCircuit *circuitBreaker
}

func main() {
Expand Down Expand Up @@ -255,7 +257,7 @@
}),
retry.Context(ctx),
)
if err != nil {

Check failure on line 260 in cmd/goose/main.go

View workflow job for this annotation

GitHub Actions / golangci-lint

ifElseChain: rewrite if-else to switch statement (gocritic)
slog.Warn("Failed to load current user after retries", "maxRetries", maxRetries, "error", err)
if app.authError == "" {
app.authError = fmt.Sprintf("Failed to load user: %v", err)
Expand Down Expand Up @@ -285,7 +287,7 @@
})
}

func (app *App) onReady(ctx context.Context) {

Check failure on line 290 in cmd/goose/main.go

View workflow job for this annotation

GitHub Actions / golangci-lint

cognitive complexity 59 of func `(*App).onReady` is high (> 55) (gocognit)
slog.Info("System tray ready")

// On Linux, immediately build a minimal menu to ensure it's visible
Expand Down Expand Up @@ -613,7 +615,7 @@
"outgoing_count", len(outgoing))
// Log ALL outgoing PRs for debugging
slog.Debug("[UPDATE] Listing ALL outgoing PRs for debugging")
for i, pr := range outgoing {

Check failure on line 618 in cmd/goose/main.go

View workflow job for this annotation

GitHub Actions / golangci-lint

rangeValCopy: each iteration copies 200 bytes (consider pointers or indexing) (gocritic)
slog.Debug("[UPDATE] Outgoing PR details",
"index", i,
"repo", pr.Repository,
Expand Down Expand Up @@ -800,7 +802,7 @@
}

// tryAutoOpenPR attempts to open a PR in the browser if enabled and rate limits allow.
func (app *App) tryAutoOpenPR(ctx context.Context, pr PR, autoBrowserEnabled bool, startTime time.Time) {

Check failure on line 805 in cmd/goose/main.go

View workflow job for this annotation

GitHub Actions / golangci-lint

hugeParam: pr is heavy (200 bytes); consider passing it by pointer (gocritic)
if !autoBrowserEnabled {
return
}
Expand Down
1 change: 0 additions & 1 deletion cmd/goose/notifications.go
Original file line number Diff line number Diff line change
Expand Up @@ -89,7 +89,6 @@
app.tryAutoOpenPR(ctx, pr, app.enableAutoBrowser, app.startTime)
}
}

}()

// Update menu immediately after sending notifications
Expand All @@ -102,7 +101,7 @@
}

// sendPRNotification sends a notification for a single PR.
func (app *App) sendPRNotification(ctx context.Context, pr PR, title string, soundType string, playedSound *bool) {

Check failure on line 104 in cmd/goose/notifications.go

View workflow job for this annotation

GitHub Actions / golangci-lint

hugeParam: pr is heavy (200 bytes); consider passing it by pointer (gocritic)
message := fmt.Sprintf("%s #%d: %s", pr.Repository, pr.Number, pr.Title)

// Send desktop notification in a goroutine to avoid blocking
Expand Down
Loading