Merge pull request #30 from codeGROOVE-dev/auto-open

Add feature for auto-opening incoming PRs, remove reminder feature

Author: tstromberg

README.md

@@ -18,6 +18,7 @@ Lives in your menubar like a tiny waterfowl of productivity shame, watching your
 - **✈️ Jet sounds** when your own PR is ready for the next stage
 - **🧠 Smart turn-based assignment** - knows who is blocking a PR, knows when tests are failing, etc.
 - **⭐ Auto-start** on login (macOS)
+- **🔔 Auto-open** incoming PRs in your browser (off by default, rate-limited)
 
 You can also visit the web-based equivalent at https://dash.ready-to-review.dev/
 

cmd/goose/browser_rate_limiter.go

@@ -0,0 +1,119 @@
+// Package main implements browser rate limiting for PR auto-open feature.
+package main
+
+import (
+	"log"
+	"sync"
+	"time"
+)
+
+// BrowserRateLimiter manages rate limiting for automatically opening browser windows.
+type BrowserRateLimiter struct {
+	openedPRs        map[string]bool
+	openedLastMinute []time.Time
+	openedToday      []time.Time
+	startupDelay     time.Duration
+	maxPerMinute     int
+	maxPerDay        int
+	mu               sync.Mutex
+}
+
+// NewBrowserRateLimiter creates a new browser rate limiter.
+func NewBrowserRateLimiter(startupDelay time.Duration, maxPerMinute, maxPerDay int) *BrowserRateLimiter {
+	log.Printf("[BROWSER] Initializing rate limiter: startup_delay=%v, max_per_minute=%d, max_per_day=%d",
+		startupDelay, maxPerMinute, maxPerDay)
+	return &BrowserRateLimiter{
+		openedLastMinute: make([]time.Time, 0),
+		openedToday:      make([]time.Time, 0),
+		startupDelay:     startupDelay,
+		maxPerMinute:     maxPerMinute,
+		maxPerDay:        maxPerDay,
+		openedPRs:        make(map[string]bool),
+	}
+}
+
+// CanOpen checks if we can open a browser window according to rate limits.
+func (b *BrowserRateLimiter) CanOpen(startTime time.Time, prURL string) bool {
+	b.mu.Lock()
+	defer b.mu.Unlock()
+
+	// Check if we've already opened this PR
+	if b.openedPRs[prURL] {
+		log.Printf("[BROWSER] Skipping auto-open: PR already opened - %s", prURL)
+		return false
+	}
+
+	// Check startup delay
+	if time.Since(startTime) < b.startupDelay {
+		log.Printf("[BROWSER] Skipping auto-open: within startup delay period (%v remaining)",
+			b.startupDelay-time.Since(startTime))
+		return false
+	}
+
+	now := time.Now()
+
+	// Clean old entries
+	b.cleanOldEntries(now)
+
+	// Check per-minute limit
+	if len(b.openedLastMinute) >= b.maxPerMinute {
+		log.Printf("[BROWSER] Rate limit: already opened %d PRs in the last minute (max: %d)",
+			len(b.openedLastMinute), b.maxPerMinute)
+		return false
+	}
+
+	// Check per-day limit
+	if len(b.openedToday) >= b.maxPerDay {
+		log.Printf("[BROWSER] Rate limit: already opened %d PRs today (max: %d)",
+			len(b.openedToday), b.maxPerDay)
+		return false
+	}
+
+	return true
+}
+
+// RecordOpen records that a browser window was opened.
+func (b *BrowserRateLimiter) RecordOpen(prURL string) {
+	b.mu.Lock()
+	defer b.mu.Unlock()
+
+	now := time.Now()
+	b.openedLastMinute = append(b.openedLastMinute, now)
+	b.openedToday = append(b.openedToday, now)
+	b.openedPRs[prURL] = true
+
+	log.Printf("[BROWSER] Recorded browser open for %s (minute: %d/%d, today: %d/%d)",
+		prURL, len(b.openedLastMinute), b.maxPerMinute, len(b.openedToday), b.maxPerDay)
+}
+
+// cleanOldEntries removes entries outside the time windows.
+func (b *BrowserRateLimiter) cleanOldEntries(now time.Time) {
+	// Clean entries older than 1 minute
+	oneMinuteAgo := now.Add(-1 * time.Minute)
+	newLastMinute := make([]time.Time, 0, len(b.openedLastMinute))
+	for _, t := range b.openedLastMinute {
+		if t.After(oneMinuteAgo) {
+			newLastMinute = append(newLastMinute, t)
+		}
+	}
+	b.openedLastMinute = newLastMinute
+
+	// Clean entries older than 24 hours (1 day)
+	oneDayAgo := now.Add(-24 * time.Hour)
+	newToday := make([]time.Time, 0, len(b.openedToday))
+	for _, t := range b.openedToday {
+		if t.After(oneDayAgo) {
+			newToday = append(newToday, t)
+		}
+	}
+	b.openedToday = newToday
+}
+
+// Reset clears the opened PRs tracking - useful when toggling the feature.
+func (b *BrowserRateLimiter) Reset() {
+	b.mu.Lock()
+	defer b.mu.Unlock()
+	previousCount := len(b.openedPRs)
+	b.openedPRs = make(map[string]bool)
+	log.Printf("[BROWSER] Rate limiter reset: cleared %d tracked PRs", previousCount)
+}

cmd/goose/main.go

@@ -44,6 +44,7 @@ const (
 	panicFailureIncrement     = 10
 	turnAPITimeout            = 10 * time.Second
 	maxConcurrentTurnAPICalls = 10
+	defaultMaxBrowserOpensDay = 20
 )
 
 // PR represents a pull request with metadata.
@@ -90,8 +91,10 @@ type App struct {
 	loadingTurnData     bool
 	menuInitialized     bool
 	initialLoadComplete bool
-	enableReminders     bool
 	enableAudioCues     bool
+	enableAutoBrowser   bool
+	browserRateLimiter  *BrowserRateLimiter
+	startTime           time.Time
 }
 
 func loadCurrentUser(ctx context.Context, app *App) {
@@ -145,9 +148,15 @@ func main() {
 	var targetUser string
 	var noCache bool
 	var updateInterval time.Duration
+	var browserOpenDelay time.Duration
+	var maxBrowserOpensMinute int
+	var maxBrowserOpensDay int
 	flag.StringVar(&targetUser, "user", "", "GitHub user to query PRs for (defaults to authenticated user)")
 	flag.BoolVar(&noCache, "no-cache", false, "Bypass cache for debugging")
 	flag.DurationVar(&updateInterval, "interval", defaultUpdateInterval, "Update interval (e.g. 30s, 1m, 5m)")
+	flag.DurationVar(&browserOpenDelay, "browser-delay", 1*time.Minute, "Minimum delay before opening PRs in browser after startup")
+	flag.IntVar(&maxBrowserOpensMinute, "browser-max-per-minute", 2, "Maximum browser windows to open per minute")
+	flag.IntVar(&maxBrowserOpensDay, "browser-max-per-day", defaultMaxBrowserOpensDay, "Maximum browser windows to open per day")
 	flag.Parse()
 
 	// Validate target user if provided
@@ -163,9 +172,25 @@ func main() {
 		updateInterval = minUpdateInterval
 	}
 
+	// Validate browser rate limit parameters
+	if maxBrowserOpensMinute < 0 {
+		log.Printf("Invalid browser-max-per-minute %d, using default of 2", maxBrowserOpensMinute)
+		maxBrowserOpensMinute = 2
+	}
+	if maxBrowserOpensDay < 0 {
+		log.Printf("Invalid browser-max-per-day %d, using default of %d", maxBrowserOpensDay, defaultMaxBrowserOpensDay)
+		maxBrowserOpensDay = defaultMaxBrowserOpensDay
+	}
+	if browserOpenDelay < 0 {
+		log.Printf("Invalid browser-delay %v, using default of 1 minute", browserOpenDelay)
+		browserOpenDelay = 1 * time.Minute
+	}
+
 	log.SetFlags(log.LstdFlags | log.Lshortfile)
 	log.Printf("Starting GitHub PR Monitor (version=%s, commit=%s, date=%s)", version, commit, date)
 	log.Printf("Configuration: update_interval=%v, max_retries=%d, max_delay=%v", updateInterval, maxRetries, maxRetryDelay)
+	log.Printf("Browser auto-open: startup_delay=%v, max_per_minute=%d, max_per_day=%d",
+		browserOpenDelay, maxBrowserOpensMinute, maxBrowserOpensDay)
 
 	ctx := context.Background()
 
@@ -187,8 +212,10 @@ func main() {
 		noCache:            noCache,
 		updateInterval:     updateInterval,
 		pendingTurnResults: make([]TurnResult, 0),
-		enableReminders:    true,
 		enableAudioCues:    true,
+		enableAutoBrowser:  false, // Default to false for safety
+		browserRateLimiter: NewBrowserRateLimiter(browserOpenDelay, maxBrowserOpensMinute, maxBrowserOpensDay),
+		startTime:          time.Now(),
 	}
 
 	// Load saved settings
@@ -492,6 +519,26 @@ func (app *App) updatePRsWithWait(ctx context.Context) {
 	app.checkForNewlyBlockedPRs(ctx)
 }
 
+// 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) {
+	if !autoBrowserEnabled {
+		return
+	}
+
+	if app.browserRateLimiter.CanOpen(startTime, pr.URL) {
+		log.Printf("[BROWSER] Auto-opening newly blocked PR: %s #%d - %s",
+			pr.Repository, pr.Number, pr.URL)
+		// Use strict validation for auto-opened URLs
+		if err := openURLAutoStrict(ctx, pr.URL); err != nil {
+			log.Printf("[BROWSER] Failed to auto-open PR %s: %v", pr.URL, err)
+		} else {
+			app.browserRateLimiter.RecordOpen(pr.URL)
+			log.Printf("[BROWSER] Successfully opened PR %s #%d in browser",
+				pr.Repository, pr.Number)
+		}
+	}
+}
+
 // notifyWithSound sends a notification and plays sound only once per cycle.
 func (app *App) notifyWithSound(ctx context.Context, pr PR, isIncoming bool, playedSound *bool) {
 	var title, soundType string
@@ -521,6 +568,8 @@ func (app *App) checkForNewlyBlockedPRs(ctx context.Context) {
 	incoming := app.incoming
 	outgoing := app.outgoing
 	previousBlocked := app.previousBlockedPRs
+	autoBrowserEnabled := app.enableAutoBrowser
+	startTime := app.startTime
 	app.mu.RUnlock()
 
 	currentBlocked := make(map[string]bool)
@@ -534,6 +583,7 @@ func (app *App) checkForNewlyBlockedPRs(ctx context.Context) {
 			// Notify if newly blocked
 			if !previousBlocked[incoming[i].URL] {
 				app.notifyWithSound(ctx, incoming[i], true, &playedHonk)
+				app.tryAutoOpenPR(ctx, incoming[i], autoBrowserEnabled, startTime)
 			}
 		}
 	}
@@ -549,6 +599,7 @@ func (app *App) checkForNewlyBlockedPRs(ctx context.Context) {
 					time.Sleep(2 * time.Second)
 				}
 				app.notifyWithSound(ctx, outgoing[i], false, &playedJet)
+				app.tryAutoOpenPR(ctx, outgoing[i], autoBrowserEnabled, startTime)
 			}
 		}
 	}

cmd/goose/security.go

@@ -29,6 +29,11 @@ var (
 	// New tokens: ghp_ (personal), ghs_ (server), ghr_ (refresh), gho_ (OAuth), ghu_ (user-to-server) followed by base62 chars.
 	// Fine-grained tokens: github_pat_ followed by base62 chars.
 	githubTokenRegex = regexp.MustCompile(`^[a-f0-9]{40}$|^gh[psoru]_[A-Za-z0-9]{36,251}$|^github_pat_[A-Za-z0-9]{82}$`)
+
+	// githubPRURLRegex validates strict GitHub PR URL format for auto-opening.
+	// Must match: https://github.com/{owner}/{repo}/pull/{number}
+	// Owner and repo follow GitHub naming rules, number is digits only.
+	githubPRURLRegex = regexp.MustCompile(`^https://github\.com/[a-zA-Z0-9][a-zA-Z0-9-]{0,38}/[a-zA-Z0-9][a-zA-Z0-9._-]{0,99}/pull/[1-9][0-9]{0,9}$`)
 )
 
 // validateGitHubUsername validates a GitHub username.
@@ -115,3 +120,54 @@ func validateURL(rawURL string) error {
 
 	return nil
 }
+
+// validateGitHubPRURL performs strict validation for GitHub PR URLs used in auto-opening.
+// This ensures the URL follows the exact pattern: https://github.com/{owner}/{repo}/pull/{number}
+// with no additional path segments, fragments, or suspicious characters.
+// The URL may optionally have ?goose=1 parameter which we add for tracking.
+func validateGitHubPRURL(rawURL string) error {
+	// First do basic URL validation
+	if err := validateURL(rawURL); err != nil {
+		return err
+	}
+
+	// Strip the ?goose=1 parameter if present for pattern validation
+	urlToValidate := rawURL
+	if strings.HasSuffix(rawURL, "?goose=1") {
+		urlToValidate = strings.TrimSuffix(rawURL, "?goose=1")
+	}
+
+	// Check against strict GitHub PR URL pattern
+	if !githubPRURLRegex.MatchString(urlToValidate) {
+		return fmt.Errorf("URL does not match GitHub PR pattern: %s", urlToValidate)
+	}
+
+	// Additional security checks
+	// Reject URLs with @ (potential credential injection)
+	if strings.Contains(rawURL, "@") {
+		return errors.New("URL contains @ character")
+	}
+
+	// Reject URLs with URL encoding (could hide malicious content)  
+	// Exception: %3D which is = in URL encoding, only as part of ?goose=1
+	if strings.Contains(rawURL, "%") && !strings.HasSuffix(rawURL, "?goose%3D1") {
+		return errors.New("URL contains encoded characters")
+	}
+
+	// Reject URLs with fragments
+	if strings.Contains(rawURL, "#") {
+		return errors.New("URL contains fragments")
+	}
+
+	// Allow only ?goose=1 query parameter, nothing else
+	if strings.Contains(rawURL, "?") && !strings.HasSuffix(rawURL, "?goose=1") && !strings.HasSuffix(rawURL, "?goose%3D1") {
+		return errors.New("URL contains unexpected query parameters")
+	}
+
+	// Reject URLs with double slashes (except after https:)
+	if strings.Contains(rawURL[8:], "//") {
+		return errors.New("URL contains double slashes")
+	}
+
+	return nil
+}