merge up

Author: tstromberg

cmd/goose/cache.go

@@ -35,6 +35,13 @@ func (app *App) turnData(ctx context.Context, url string, updatedAt time.Time) (
 	hash := sha256.Sum256([]byte(key))
 	cacheFile := filepath.Join(app.cacheDir, hex.EncodeToString(hash[:])[:16]+".json")
 
+	// Log the cache key details
+	slog.Debug("[CACHE] Checking cache",
+		"url", url,
+		"updated_at", updatedAt.Format(time.RFC3339),
+		"cache_key", key,
+		"cache_file", filepath.Base(cacheFile))
+
 	// Skip cache if --no-cache flag is set
 	if !app.noCache {
 		// Try to read from cache (gracefully handle all cache errors)
@@ -47,17 +54,46 @@ func (app *App) turnData(ctx context.Context, url string, updatedAt time.Time) (
 					slog.Error("Failed to remove corrupted cache file", "error", removeErr)
 				}
 			} else if time.Since(entry.CachedAt) < cacheTTL && entry.UpdatedAt.Equal(updatedAt) {
-				// Check if cache is still valid (2 hour TTL)
+				// 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)
+				}
 				return entry.Data, true, nil
+			} else {
+				// Log why cache was invalid
+				if !entry.UpdatedAt.Equal(updatedAt) {
+					slog.Debug("[CACHE] Cache miss - PR updated",
+						"url", url,
+						"cached_pr_time", entry.UpdatedAt.Format(time.RFC3339),
+						"current_pr_time", updatedAt.Format(time.RFC3339))
+				} else if time.Since(entry.CachedAt) >= cacheTTL {
+					slog.Debug("[CACHE] Cache miss - TTL expired",
+						"url", url,
+						"cached_at", entry.CachedAt.Format(time.RFC3339),
+						"cache_age", time.Since(entry.CachedAt).Round(time.Second),
+						"ttl", cacheTTL)
+				}
 			}
+		} else if !os.IsNotExist(readErr) {
+			slog.Debug("[CACHE] Cache file read error", "url", url, "error", readErr)
 		}
 	}
 
 	// Cache miss, fetch from API
 	if app.noCache {
 		slog.Debug("Cache bypassed (--no-cache), fetching from Turn API", "url", url)
 	} else {
-		slog.Debug("Cache miss, fetching from Turn API", "url", url)
+		slog.Info("[CACHE] Cache miss, fetching from Turn API",
+			"url", url,
+			"pr_updated_at", updatedAt.Format(time.RFC3339))
+		if app.healthMonitor != nil {
+			app.healthMonitor.recordCacheAccess(false)
+		}
 	}
 
 	// Use exponential backoff with jitter for Turn API calls
@@ -68,11 +104,16 @@ func (app *App) turnData(ctx context.Context, url string, updatedAt time.Time) (
 		defer cancel()
 
 		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)
 		if retryErr != nil {
 			slog.Warn("Turn API error (will retry)", "error", retryErr)
 			return retryErr
 		}
+		slog.Debug("[TURN] API call successful", "url", url)
 		return nil
 	},
 		retry.Attempts(maxRetries),
@@ -85,9 +126,16 @@ func (app *App) turnData(ctx context.Context, url string, updatedAt time.Time) (
 	)
 	if err != nil {
 		slog.Error("Turn API error after retries (will use PR without metadata)", "maxRetries", maxRetries, "error", err)
+		if app.healthMonitor != nil {
+			app.healthMonitor.recordAPICall(false)
+		}
 		return nil, false, err
 	}
 
+	if app.healthMonitor != nil {
+		app.healthMonitor.recordAPICall(true)
+	}
+
 	// Save to cache (don't fail if caching fails) - skip if --no-cache is set
 	if !app.noCache {
 		entry := cacheEntry{
@@ -103,14 +151,20 @@ func (app *App) turnData(ctx context.Context, url string, updatedAt time.Time) (
 				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))
 			}
 		}
 	}
 
 	return data, false, nil
 }
 
-// cleanupOldCache removes cache files older than 5 days.
+// cleanupOldCache removes cache files older than the cleanup interval (15 days).
 func (app *App) cleanupOldCache() {
 	entries, err := os.ReadDir(app.cacheDir)
 	if err != nil {
@@ -131,7 +185,7 @@ func (app *App) cleanupOldCache() {
 			continue
 		}
 
-		// Remove cache files older than 5 days
+		// Remove cache files older than cleanup interval (15 days)
 		if time.Since(info.ModTime()) > cacheCleanupInterval {
 			filePath := filepath.Join(app.cacheDir, entry.Name())
 			if removeErr := os.Remove(filePath); removeErr != nil {

cmd/goose/deadlock_test.go

@@ -14,7 +14,7 @@ func TestConcurrentMenuOperations(t *testing.T) {
 		hiddenOrgs:         make(map[string]bool),
 		seenOrgs:           make(map[string]bool),
 		blockedPRTimes:     make(map[string]time.Time),
-		browserRateLimiter: NewBrowserRateLimiter(30*time.Second, 5, defaultMaxBrowserOpensDay),
+		browserRateLimiter: NewBrowserRateLimiter(startupGracePeriod, 5, defaultMaxBrowserOpensDay),
 		systrayInterface:   &MockSystray{},
 		incoming: []PR{
 			{Repository: "org1/repo1", Number: 1, Title: "Fix bug", URL: "https://github.com/org1/repo1/pull/1"},
@@ -101,7 +101,7 @@ func TestMenuClickDeadlockScenario(t *testing.T) {
 		hiddenOrgs:         make(map[string]bool),
 		seenOrgs:           make(map[string]bool),
 		blockedPRTimes:     make(map[string]time.Time),
-		browserRateLimiter: NewBrowserRateLimiter(30*time.Second, 5, defaultMaxBrowserOpensDay),
+		browserRateLimiter: NewBrowserRateLimiter(startupGracePeriod, 5, defaultMaxBrowserOpensDay),
 		systrayInterface:   &MockSystray{},
 		incoming: []PR{
 			{Repository: "org1/repo1", Number: 1, Title: "Test PR", URL: "https://github.com/org1/repo1/pull/1"},
@@ -142,7 +142,7 @@ func TestRapidMenuClicks(t *testing.T) {
 		hiddenOrgs:         make(map[string]bool),
 		seenOrgs:           make(map[string]bool),
 		blockedPRTimes:     make(map[string]time.Time),
-		browserRateLimiter: NewBrowserRateLimiter(30*time.Second, 5, defaultMaxBrowserOpensDay),
+		browserRateLimiter: NewBrowserRateLimiter(startupGracePeriod, 5, defaultMaxBrowserOpensDay),
 		systrayInterface:   &MockSystray{},
 		lastSearchAttempt:  time.Now().Add(-15 * time.Second), // Allow first click
 		incoming: []PR{

cmd/goose/github.go

@@ -22,11 +22,14 @@ import (
 
 // extractOrgFromRepo extracts the organization name from a repository path like "org/repo".
 func extractOrgFromRepo(repo string) string {
-	parts := strings.Split(repo, "/")
-	if len(parts) >= 1 {
-		return parts[0]
+	idx := strings.Index(repo, "/")
+	if idx > 0 {
+		return repo[:idx]
 	}
-	return ""
+	if idx == 0 {
+		return "" // Invalid: starts with "/"
+	}
+	return repo // No slash: return as-is (single segment or empty)
 }
 
 // initClients initializes GitHub and Turn API clients.
@@ -180,25 +183,42 @@ func (app *App) executeGitHubQuery(ctx context.Context, query string, opts *gith
 	var result *github.IssuesSearchResult
 	var resp *github.Response
 
-	err := retry.Do(func() error {
+	// Use circuit breaker if available
+	if app.githubCircuit != nil {
+		err := app.githubCircuit.call(func() error {
+			return app.executeGitHubQueryInternal(ctx, query, opts, &result, &resp)
+		})
+		if err != nil {
+			return nil, err
+		}
+		return result, nil
+	}
+
+	// Fallback to direct execution
+	err := app.executeGitHubQueryInternal(ctx, query, opts, &result, &resp)
+	return result, err
+}
+
+func (app *App) executeGitHubQueryInternal(ctx context.Context, query string, opts *github.SearchOptions, result **github.IssuesSearchResult, resp **github.Response) error {
+	return 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)
+		*result, *resp, retryErr = app.client.Search.Issues(githubCtx, query, opts)
 		if retryErr != nil {
 			// Enhanced error handling with specific cases
-			if resp != nil {
+			if *resp != nil {
 				const (
 					httpStatusUnauthorized  = 401
 					httpStatusForbidden     = 403
 					httpStatusUnprocessable = 422
 				)
-				switch resp.StatusCode {
+				switch (*resp).StatusCode {
 				case httpStatusForbidden:
-					if resp.Header.Get("X-Ratelimit-Remaining") == "0" {
-						resetTime := resp.Header.Get("X-Ratelimit-Reset")
+					if (*resp).Header.Get("X-Ratelimit-Remaining") == "0" {
+						resetTime := (*resp).Header.Get("X-Ratelimit-Reset")
 						slog.Warn("GitHub API rate limited (will retry)", "resetTime", resetTime)
 						return retryErr // Retry on rate limit
 					}
@@ -211,7 +231,7 @@ func (app *App) executeGitHubQuery(ctx context.Context, query string, opts *gith
 					slog.Error("GitHub API query invalid", "query", query)
 					return retry.Unrecoverable(fmt.Errorf("github API query invalid: %w", retryErr))
 				default:
-					slog.Warn("GitHub API error (will retry)", "statusCode", resp.StatusCode, "error", retryErr)
+					slog.Warn("GitHub API error (will retry)", "statusCode", (*resp).StatusCode, "error", retryErr)
 				}
 			} else {
 				// Likely network error - retry these
@@ -229,10 +249,6 @@ func (app *App) executeGitHubQuery(ctx context.Context, query string, opts *gith
 		}),
 		retry.Context(ctx),
 	)
-	if err != nil {
-		return nil, err
-	}
-	return result, nil
 }
 
 // prResult holds the result of a Turn API query for a PR.
@@ -371,8 +387,10 @@ func (app *App) fetchPRsInternal(ctx context.Context) (incoming []PR, outgoing [
 			Title:      issue.GetTitle(),
 			URL:        issue.GetHTMLURL(),
 			Repository: repo,
+			Author:     issue.GetUser().GetLogin(),
 			Number:     issue.GetNumber(),
 			UpdatedAt:  issue.GetUpdatedAt().Time,
+			IsDraft:    issue.GetDraft(),
 		}
 
 		// Categorize as incoming or outgoing
@@ -477,22 +495,24 @@ func (app *App) fetchTurnDataSync(ctx context.Context, issues []*github.Issue, u
 			// Update the PR in the slices directly
 			if result.isOwner {
 				for i := range *outgoing {
-					if (*outgoing)[i].URL == result.url {
-						(*outgoing)[i].NeedsReview = needsReview
-						(*outgoing)[i].IsBlocked = isBlocked
-						(*outgoing)[i].ActionReason = actionReason
-						(*outgoing)[i].ActionKind = actionKind
-						break
+					if (*outgoing)[i].URL != result.url {
+						continue
 					}
+					(*outgoing)[i].NeedsReview = needsReview
+					(*outgoing)[i].IsBlocked = isBlocked
+					(*outgoing)[i].ActionReason = actionReason
+					(*outgoing)[i].ActionKind = actionKind
+					break
 				}
 			} else {
 				for i := range *incoming {
-					if (*incoming)[i].URL == result.url {
-						(*incoming)[i].NeedsReview = needsReview
-						(*incoming)[i].ActionReason = actionReason
-						(*incoming)[i].ActionKind = actionKind
-						break
+					if (*incoming)[i].URL != result.url {
+						continue
 					}
+					(*incoming)[i].NeedsReview = needsReview
+					(*incoming)[i].ActionReason = actionReason
+					(*incoming)[i].ActionKind = actionKind
+					break
 				}
 			}
 		} else if result.err != nil {