Skip to content

Commit 6c6ce47

Browse files
Thomas StrombergThomas Stromberg
Thomas Stromberg
authored and
Thomas Stromberg
committed
Bypass caching for PRs with pending tests
1 parent 707706a commit 6c6ce47

File tree

6 files changed

+112
-69
lines changed

6 files changed

+112
-69
lines changed

‎cmd/goose/cache.go‎

Lines changed: 66 additions & 27 deletions
Original file line numberDiff line numberDiff line change
@@ -25,6 +25,8 @@ type cacheEntry struct {
2525

2626
// turnData fetches Turn API data with caching.
2727
func (app *App) turnData(ctx context.Context, url string, updatedAt time.Time) (*turn.CheckResponse, bool, error) {
28+
prAge := time.Since(updatedAt)
29+
hasRunningTests := false
2830
// Validate URL before processing
2931
if err := validateURL(url); err != nil {
3032
return nil, false, fmt.Errorf("invalid URL: %w", err)
@@ -55,15 +57,26 @@ func (app *App) turnData(ctx context.Context, url string, updatedAt time.Time) (
5557
}
5658
} else if time.Since(entry.CachedAt) < cacheTTL && entry.UpdatedAt.Equal(updatedAt) {
5759
// Check if cache is still valid (10 day TTL, but PR UpdatedAt is primary check)
58-
slog.Debug("[CACHE] Cache hit",
59-
"url", url,
60-
"cached_at", entry.CachedAt.Format(time.RFC3339),
61-
"cache_age", time.Since(entry.CachedAt).Round(time.Second),
62-
"pr_updated_at", entry.UpdatedAt.Format(time.RFC3339))
63-
if app.healthMonitor != nil {
64-
app.healthMonitor.recordCacheAccess(true)
60+
// But invalidate cache for PRs with running tests if they're fresh (< 90 minutes old)
61+
if entry.Data != nil && entry.Data.PullRequest.TestState == "running" && prAge < runningTestsCacheBypass {
62+
hasRunningTests = true
63+
slog.Debug("[CACHE] Cache invalidated - PR has running tests and is fresh",
64+
"url", url,
65+
"test_state", entry.Data.PullRequest.TestState,
66+
"pr_age", prAge.Round(time.Minute),
67+
"cached_at", entry.CachedAt.Format(time.RFC3339))
68+
// Don't return cached data - fall through to fetch fresh data with current time
69+
} else {
70+
slog.Debug("[CACHE] Cache hit",
71+
"url", url,
72+
"cached_at", entry.CachedAt.Format(time.RFC3339),
73+
"cache_age", time.Since(entry.CachedAt).Round(time.Second),
74+
"pr_updated_at", entry.UpdatedAt.Format(time.RFC3339))
75+
if app.healthMonitor != nil {
76+
app.healthMonitor.recordCacheAccess(true)
77+
}
78+
return entry.Data, true, nil
6579
}
66-
return entry.Data, true, nil
6780
} else {
6881
// Log why cache was invalid
6982
if !entry.UpdatedAt.Equal(updatedAt) {
@@ -103,12 +116,22 @@ func (app *App) turnData(ctx context.Context, url string, updatedAt time.Time) (
103116
turnCtx, cancel := context.WithTimeout(ctx, turnAPITimeout)
104117
defer cancel()
105118

119+
// For PRs with running tests, send current time to bypass Turn server cache
120+
timestampToSend := updatedAt
121+
if hasRunningTests {
122+
timestampToSend = time.Now()
123+
slog.Debug("[TURN] Using current timestamp for PR with running tests to bypass Turn server cache",
124+
"url", url,
125+
"pr_updated_at", updatedAt.Format(time.RFC3339),
126+
"timestamp_sent", timestampToSend.Format(time.RFC3339))
127+
}
128+
106129
var retryErr error
107130
slog.Debug("[TURN] Making API call",
108131
"url", url,
109132
"user", app.currentUser.GetLogin(),
110-
"pr_updated_at", updatedAt.Format(time.RFC3339))
111-
data, retryErr = app.turnClient.Check(turnCtx, url, app.currentUser.GetLogin(), updatedAt)
133+
"pr_updated_at", timestampToSend.Format(time.RFC3339))
134+
data, retryErr = app.turnClient.Check(turnCtx, url, app.currentUser.GetLogin(), timestampToSend)
112135
if retryErr != nil {
113136
slog.Warn("Turn API error (will retry)", "error", retryErr)
114137
return retryErr
@@ -137,26 +160,42 @@ func (app *App) turnData(ctx context.Context, url string, updatedAt time.Time) (
137160
}
138161

139162
// Save to cache (don't fail if caching fails) - skip if --no-cache is set
163+
// Also skip caching if tests are running and PR is fresh (updated in last 90 minutes)
140164
if !app.noCache {
141-
entry := cacheEntry{
142-
Data: data,
143-
CachedAt: time.Now(),
144-
UpdatedAt: updatedAt,
165+
shouldCache := true
166+
prAge := time.Since(updatedAt)
167+
168+
// Don't cache PRs with running tests unless they're older than 90 minutes
169+
if data != nil && data.PullRequest.TestState == "running" && prAge < runningTestsCacheBypass {
170+
shouldCache = false
171+
slog.Debug("[CACHE] Skipping cache for PR with running tests",
172+
"url", url,
173+
"test_state", data.PullRequest.TestState,
174+
"pr_age", prAge.Round(time.Minute),
175+
"pending_checks", len(data.PullRequest.CheckSummary.PendingStatuses))
145176
}
146-
if cacheData, marshalErr := json.Marshal(entry); marshalErr != nil {
147-
slog.Error("Failed to marshal cache data", "url", url, "error", marshalErr)
148-
} else {
149-
// Ensure cache directory exists with secure permissions
150-
if dirErr := os.MkdirAll(filepath.Dir(cacheFile), 0o700); dirErr != nil {
151-
slog.Error("Failed to create cache directory", "error", dirErr)
152-
} else if writeErr := os.WriteFile(cacheFile, cacheData, 0o600); writeErr != nil {
153-
slog.Error("Failed to write cache file", "error", writeErr)
177+
178+
if shouldCache {
179+
entry := cacheEntry{
180+
Data: data,
181+
CachedAt: time.Now(),
182+
UpdatedAt: updatedAt,
183+
}
184+
if cacheData, marshalErr := json.Marshal(entry); marshalErr != nil {
185+
slog.Error("Failed to marshal cache data", "url", url, "error", marshalErr)
154186
} else {
155-
slog.Debug("[CACHE] Saved to cache",
156-
"url", url,
157-
"cached_at", entry.CachedAt.Format(time.RFC3339),
158-
"pr_updated_at", entry.UpdatedAt.Format(time.RFC3339),
159-
"cache_file", filepath.Base(cacheFile))
187+
// Ensure cache directory exists with secure permissions
188+
if dirErr := os.MkdirAll(filepath.Dir(cacheFile), 0o700); dirErr != nil {
189+
slog.Error("Failed to create cache directory", "error", dirErr)
190+
} else if writeErr := os.WriteFile(cacheFile, cacheData, 0o600); writeErr != nil {
191+
slog.Error("Failed to write cache file", "error", writeErr)
192+
} else {
193+
slog.Debug("[CACHE] Saved to cache",
194+
"url", url,
195+
"cached_at", entry.CachedAt.Format(time.RFC3339),
196+
"pr_updated_at", entry.UpdatedAt.Format(time.RFC3339),
197+
"cache_file", filepath.Base(cacheFile))
198+
}
160199
}
161200
}
162201
}

‎cmd/goose/github.go‎

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -481,6 +481,7 @@ func (app *App) fetchTurnDataSync(ctx context.Context, issues []*github.Issue, u
481481
isBlocked := false
482482
actionReason := ""
483483
actionKind := ""
484+
testState := result.turnData.PullRequest.TestState
484485
if action, exists := result.turnData.Analysis.NextAction[user]; exists {
485486
needsReview = true
486487
isBlocked = action.Critical // Only critical actions are blocking
@@ -502,6 +503,7 @@ func (app *App) fetchTurnDataSync(ctx context.Context, issues []*github.Issue, u
502503
(*outgoing)[i].IsBlocked = isBlocked
503504
(*outgoing)[i].ActionReason = actionReason
504505
(*outgoing)[i].ActionKind = actionKind
506+
(*outgoing)[i].TestState = testState
505507
break
506508
}
507509
} else {
@@ -512,6 +514,7 @@ func (app *App) fetchTurnDataSync(ctx context.Context, issues []*github.Issue, u
512514
(*incoming)[i].NeedsReview = needsReview
513515
(*incoming)[i].ActionReason = actionReason
514516
(*incoming)[i].ActionKind = actionKind
517+
(*incoming)[i].TestState = testState
515518
break
516519
}
517520
}

‎cmd/goose/icons.go‎

Lines changed: 7 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -7,10 +7,10 @@ import (
77
)
88

99
// Icon variables are defined in platform-specific files:
10-
// - icons_windows.go: uses .ico files
11-
// - icons_unix.go: uses .png files
10+
// - icons_windows.go: uses .ico files.
11+
// - icons_unix.go: uses .png files.
1212

13-
// IconType represents different icon states
13+
// IconType represents different icon states.
1414
type IconType int
1515

1616
const (
@@ -22,7 +22,7 @@ const (
2222
IconLock // Authentication error
2323
)
2424

25-
// getIcon returns the icon bytes for the given type
25+
// getIcon returns the icon bytes for the given type.
2626
func getIcon(iconType IconType) []byte {
2727
switch iconType {
2828
case IconGoose:
@@ -43,7 +43,7 @@ func getIcon(iconType IconType) []byte {
4343
}
4444
}
4545

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

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

6565
app.systrayInterface.SetIcon(iconBytes)
6666
slog.Debug("[TRAY] Setting icon", "type", iconType)
67-
}
67+
}

‎cmd/goose/icons_unix.go‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -21,4 +21,4 @@ var iconSmiling []byte
2121
var iconLock []byte
2222

2323
//go:embed icons/warning.png
24-
var iconWarning []byte
24+
var iconWarning []byte

‎cmd/goose/main.go‎

Lines changed: 35 additions & 33 deletions
Original file line numberDiff line numberDiff line change
@@ -34,6 +34,7 @@ const (
3434
cacheTTL = 10 * 24 * time.Hour // 10 days - rely mostly on PR UpdatedAt
3535
cacheCleanupInterval = 15 * 24 * time.Hour // 15 days - cleanup older than cache TTL
3636
stalePRThreshold = 90 * 24 * time.Hour
37+
runningTestsCacheBypass = 90 * time.Minute // Don't cache PRs with running tests if fresher than this
3738
maxPRsToProcess = 200
3839
minUpdateInterval = 10 * time.Second
3940
defaultUpdateInterval = 1 * time.Minute
@@ -46,7 +47,7 @@ const (
4647
turnAPITimeout = 10 * time.Second
4748
maxConcurrentTurnAPICalls = 20
4849
defaultMaxBrowserOpensDay = 20
49-
startupGracePeriod = 1 * time.Minute // Don't play sounds or auto-open for first minute
50+
startupGracePeriod = 1 * time.Minute // Don't play sounds or auto-open for first minute
5051
)
5152

5253
// PR represents a pull request with metadata.
@@ -60,6 +61,7 @@ type PR struct {
6061
Author string // GitHub username of the PR author
6162
ActionReason string
6263
ActionKind string // The kind of action expected (review, merge, fix_tests, etc.)
64+
TestState string // Test state from Turn API: "running", "passing", "failing", etc.
6365
Number int
6466
IsDraft bool
6567
IsBlocked bool
@@ -68,39 +70,39 @@ type PR struct {
6870

6971
// App holds the application state.
7072
type App struct {
71-
lastSearchAttempt time.Time
72-
lastSuccessfulFetch time.Time
73-
startTime time.Time
74-
systrayInterface SystrayInterface
75-
browserRateLimiter *BrowserRateLimiter
76-
blockedPRTimes map[string]time.Time
77-
currentUser *github.User
78-
stateManager *PRStateManager
79-
client *github.Client
80-
hiddenOrgs map[string]bool
81-
seenOrgs map[string]bool
82-
turnClient *turn.Client
83-
previousBlockedPRs map[string]bool
84-
authError string
85-
lastFetchError string
86-
cacheDir string
87-
targetUser string
88-
lastMenuTitles []string
89-
outgoing []PR
90-
incoming []PR
91-
updateInterval time.Duration
92-
consecutiveFailures int
93-
mu sync.RWMutex
94-
menuMutex sync.Mutex // Mutex to prevent concurrent menu rebuilds
95-
enableAutoBrowser bool
96-
hideStaleIncoming bool
73+
lastSearchAttempt time.Time
74+
lastSuccessfulFetch time.Time
75+
startTime time.Time
76+
systrayInterface SystrayInterface
77+
browserRateLimiter *BrowserRateLimiter
78+
blockedPRTimes map[string]time.Time
79+
currentUser *github.User
80+
stateManager *PRStateManager
81+
client *github.Client
82+
hiddenOrgs map[string]bool
83+
seenOrgs map[string]bool
84+
turnClient *turn.Client
85+
previousBlockedPRs map[string]bool
86+
authError string
87+
lastFetchError string
88+
cacheDir string
89+
targetUser string
90+
lastMenuTitles []string
91+
outgoing []PR
92+
incoming []PR
93+
updateInterval time.Duration
94+
consecutiveFailures int
95+
mu sync.RWMutex
96+
menuMutex sync.Mutex // Mutex to prevent concurrent menu rebuilds
97+
enableAutoBrowser bool
98+
hideStaleIncoming bool
9799
hasPerformedInitialDiscovery bool // Track if we've done the first poll to distinguish from real state changes
98-
noCache bool
99-
enableAudioCues bool
100-
initialLoadComplete bool
101-
menuInitialized bool
102-
healthMonitor *healthMonitor
103-
githubCircuit *circuitBreaker
100+
noCache bool
101+
enableAudioCues bool
102+
initialLoadComplete bool
103+
menuInitialized bool
104+
healthMonitor *healthMonitor
105+
githubCircuit *circuitBreaker
104106
}
105107

106108
func main() {

‎cmd/goose/notifications.go‎

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -89,7 +89,6 @@ func (app *App) processNotifications(ctx context.Context) {
8989
app.tryAutoOpenPR(ctx, pr, app.enableAutoBrowser, app.startTime)
9090
}
9191
}
92-
9392
}()
9493

9594
// Update menu immediately after sending notifications

0 commit comments

Comments
 (0)