auto-recover from auth failures

Author: tstromberg

cmd/review-goose/github.go

@@ -527,10 +527,7 @@ func (app *App) fetchTurnDataSync(ctx context.Context, issues []*github.Issue, u
 			continue
 		}
 
-		wg.Add(1)
-		go func(issue *github.Issue) {
-			defer wg.Done()
-
+		wg.Go(func() {
 			// Acquire semaphore
 			sem <- struct{}{}
 			defer func() { <-sem }()
@@ -548,7 +545,7 @@ func (app *App) fetchTurnDataSync(ctx context.Context, issues []*github.Issue, u
 				isOwner:      issue.GetUser().GetLogin() == user,
 				wasFromCache: wasFromCache,
 			}
-		}(issue)
+		})
 	}
 
 	// Close the results channel when all goroutines are done

cmd/review-goose/main.go

@@ -70,6 +70,7 @@ const (
 	maxConcurrentTurnAPICalls = 20
 	defaultMaxBrowserOpensDay = 20
 	startupGracePeriod        = 1 * time.Minute // Don't play sounds or auto-open for first minute
+	authRetryInterval         = 2 * time.Minute // Retry authentication periodically when in error state
 )
 
 // PR represents a pull request with metadata.
@@ -420,6 +421,46 @@ func (app *App) handleReauthentication(ctx context.Context) {
 	}
 }
 
+// authRetryLoop periodically attempts to re-authenticate when in auth error state.
+// This ensures the app can recover from transient auth failures without user intervention.
+func (app *App) authRetryLoop(ctx context.Context) {
+	ticker := time.NewTicker(authRetryInterval)
+	defer ticker.Stop()
+
+	slog.Info("[AUTH] Starting auth retry loop", "interval", authRetryInterval)
+
+	for {
+		select {
+		case <-ctx.Done():
+			slog.Info("[AUTH] Auth retry loop stopped (context cancelled)")
+			return
+		case <-ticker.C:
+			app.mu.RLock()
+			hasAuthError := app.authError != ""
+			app.mu.RUnlock()
+
+			if !hasAuthError {
+				slog.Info("[AUTH] Auth error cleared, stopping retry loop")
+				return
+			}
+
+			slog.Info("[AUTH] Attempting automatic re-authentication")
+			app.handleReauthentication(ctx)
+
+			// Check if we recovered
+			app.mu.RLock()
+			stillHasError := app.authError != ""
+			app.mu.RUnlock()
+
+			if !stillHasError {
+				slog.Info("[AUTH] Automatic re-authentication successful, stopping retry loop")
+				return
+			}
+			slog.Info("[AUTH] Re-authentication failed, will retry", "nextRetry", authRetryInterval)
+		}
+	}
+}
+
 func (app *App) onReady(ctx context.Context) {
 	slog.Info("System tray ready")
 
@@ -501,6 +542,8 @@ func (app *App) onReady(ctx context.Context) {
 		app.rebuildMenu(ctx)
 		// Clean old cache on startup
 		app.cleanupOldCache()
+		// Start background auth retry loop
+		go app.authRetryLoop(ctx)
 		return
 	}
 

cmd/review-goose/main_test.go

@@ -880,3 +880,154 @@ func TestNewlyBlockedPRAfterGracePeriod(t *testing.T) {
 		t.Error("Expected FirstBlockedAt to be set for newly blocked PR in state manager")
 	}
 }
+
+// TestAuthRetryLoopStopsOnSuccess tests that the auth retry loop stops when auth succeeds.
+func TestAuthRetryLoopStopsOnSuccess(t *testing.T) {
+	ctx := t.Context()
+
+	app := &App{
+		mu:               sync.RWMutex{},
+		authError:        "initial auth error",
+		systrayInterface: &MockSystray{},
+	}
+
+	// Track how many times we check authError
+	checkCount := 0
+	done := make(chan struct{})
+
+	// Start a goroutine that simulates the auth retry loop behavior
+	go func() {
+		defer close(done)
+		for {
+			select {
+			case <-ctx.Done():
+				return
+			case <-time.After(10 * time.Millisecond): // Fast ticker for testing
+				app.mu.RLock()
+				hasError := app.authError != ""
+				app.mu.RUnlock()
+
+				checkCount++
+
+				if !hasError {
+					return // Loop should exit when auth succeeds
+				}
+
+				// Simulate clearing auth error on 3rd attempt
+				if checkCount >= 3 {
+					app.mu.Lock()
+					app.authError = ""
+					app.mu.Unlock()
+				}
+			}
+		}
+	}()
+
+	// Wait for the goroutine to finish
+	select {
+	case <-done:
+		// Success - loop exited
+	case <-time.After(1 * time.Second):
+		t.Fatal("Auth retry loop did not stop after auth succeeded")
+	}
+
+	// Verify auth error was cleared
+	app.mu.RLock()
+	finalError := app.authError
+	app.mu.RUnlock()
+
+	if finalError != "" {
+		t.Errorf("Expected auth error to be cleared, got: %s", finalError)
+	}
+
+	if checkCount < 3 {
+		t.Errorf("Expected at least 3 retry attempts, got: %d", checkCount)
+	}
+}
+
+// TestAuthRetryLoopStopsOnContextCancel tests that the auth retry loop stops on context cancellation.
+func TestAuthRetryLoopStopsOnContextCancel(t *testing.T) {
+	ctx, cancel := context.WithCancel(context.Background())
+
+	app := &App{
+		mu:               sync.RWMutex{},
+		authError:        "persistent auth error",
+		systrayInterface: &MockSystray{},
+	}
+
+	done := make(chan struct{})
+
+	// Start a goroutine that simulates the auth retry loop behavior
+	go func() {
+		defer close(done)
+		ticker := time.NewTicker(10 * time.Millisecond)
+		defer ticker.Stop()
+
+		for {
+			select {
+			case <-ctx.Done():
+				return
+			case <-ticker.C:
+				app.mu.RLock()
+				hasError := app.authError != ""
+				app.mu.RUnlock()
+
+				if !hasError {
+					return
+				}
+				// Auth error persists, loop continues
+			}
+		}
+	}()
+
+	// Cancel context after a short delay
+	time.Sleep(50 * time.Millisecond)
+	cancel()
+
+	// Wait for the goroutine to finish
+	select {
+	case <-done:
+		// Success - loop exited on context cancel
+	case <-time.After(1 * time.Second):
+		t.Fatal("Auth retry loop did not stop after context cancellation")
+	}
+}
+
+// TestAuthErrorStatePreservation tests that auth error state is correctly preserved and accessible.
+func TestAuthErrorStatePreservation(t *testing.T) {
+	app := &App{
+		mu:               sync.RWMutex{},
+		systrayInterface: &MockSystray{},
+	}
+
+	// Initially no auth error
+	app.mu.RLock()
+	if app.authError != "" {
+		t.Errorf("Expected no initial auth error, got: %s", app.authError)
+	}
+	app.mu.RUnlock()
+
+	// Set auth error
+	app.mu.Lock()
+	app.authError = "token expired"
+	app.mu.Unlock()
+
+	// Verify auth error is set
+	app.mu.RLock()
+	if app.authError != "token expired" {
+		t.Errorf("Expected auth error 'token expired', got: %s", app.authError)
+	}
+	app.mu.RUnlock()
+
+	// Clear auth error
+	app.mu.Lock()
+	app.authError = ""
+	app.mu.Unlock()
+
+	// Verify auth error is cleared
+	app.mu.RLock()
+	if app.authError != "" {
+		t.Errorf("Expected auth error to be cleared, got: %s", app.authError)
+	}
+	app.mu.RUnlock()
+}