transparent test->PR lookups

Author: Thomas Stromberg

go.sum

@@ -1,16 +1,8 @@
 github.com/codeGROOVE-dev/retry v1.2.0 h1:xYpYPX2PQZmdHwuiQAGGzsBm392xIMl4nfMEFApQnu8=
 github.com/codeGROOVE-dev/retry v1.2.0/go.mod h1:8OgefgV1XP7lzX2PdKlCXILsYKuz6b4ZpHa/20iLi8E=
-golang.org/x/crypto v0.42.0 h1:chiH31gIWm57EkTXpwnqf8qeuMUi0yekh6mT2AvFlqI=
-golang.org/x/crypto v0.42.0/go.mod h1:4+rDnOTJhQCx2q7/j6rAN5XDw8kPjeaXEUR2eL94ix8=
 golang.org/x/crypto v0.43.0 h1:dduJYIi3A3KOfdGOHX8AVZ/jGiyPa3IbBozJ5kNuE04=
 golang.org/x/crypto v0.43.0/go.mod h1:BFbav4mRNlXJL4wNeejLpWxB7wMbc79PdRGhWKncxR0=
-golang.org/x/net v0.44.0 h1:evd8IRDyfNBMBTTY5XRF1vaZlD+EmWx6x8PkhR04H/I=
-golang.org/x/net v0.44.0/go.mod h1:ECOoLqd5U3Lhyeyo/QDCEVQ4sNgYsqvCZ722XogGieY=
-golang.org/x/net v0.45.0 h1:RLBg5JKixCy82FtLJpeNlVM0nrSqpCRYzVU1n8kj0tM=
-golang.org/x/net v0.45.0/go.mod h1:ECOoLqd5U3Lhyeyo/QDCEVQ4sNgYsqvCZ722XogGieY=
 golang.org/x/net v0.46.0 h1:giFlY12I07fugqwPuWJi68oOnpfqFnJIJzaIIm2JVV4=
 golang.org/x/net v0.46.0/go.mod h1:Q9BGdFy1y4nkUwiLvT5qtyhAnEHgnQ/zd8PfU6nc210=
-golang.org/x/text v0.29.0 h1:1neNs90w9YzJ9BocxfsQNHKuAT4pkghyXc4nhZ6sJvk=
-golang.org/x/text v0.29.0/go.mod h1:7MhJOA9CD2qZyOKYazxdYMF85OwPdEr9jTtBpO7ydH4=
 golang.org/x/text v0.30.0 h1:yznKA/E9zq54KzlzBEAWn1NXSQ8DIp/NYMy88xJjl4k=
 golang.org/x/text v0.30.0/go.mod h1:yDdHFIX9t+tORqspjENWgzaCVXgk0yYnYuSZ8UzzBVM=

pkg/client/client.go

@@ -11,6 +11,7 @@ import (
 	"time"
 
 	"github.com/codeGROOVE-dev/retry"
+	"github.com/codeGROOVE-dev/sprinkler/pkg/github"
 	"golang.org/x/net/websocket"
 )
 
@@ -45,8 +46,9 @@ type Event struct {
 	Timestamp  time.Time `json:"timestamp"`
 	Raw        map[string]any
 	Type       string `json:"type"`
-	URL        string `json:"url"`
+	URL        string `json:"url"` // PR URL (or repo URL for check events with race condition)
 	DeliveryID string `json:"delivery_id,omitempty"`
+	CommitSHA  string `json:"commit_sha,omitempty"` // Commit SHA for check events
 }
 
 // Config holds the configuration for the client.
@@ -89,6 +91,12 @@ type Client struct {
 	writeCh    chan any  // Channel for serializing all writes
 	eventCount int
 	retries    int
+
+	// Cache for commit SHA to PR number lookups (for check event race condition)
+	commitPRCache   map[string][]int // key: "owner/repo:sha", value: PR numbers
+	commitCacheKeys []string         // track insertion order for LRU eviction
+	cacheMu         sync.RWMutex
+	maxCacheSize    int
 }
 
 // New creates a new robust WebSocket client.
@@ -119,10 +127,13 @@ func New(config Config) (*Client, error) {
 	}
 
 	return &Client{
-		config:    config,
-		stopCh:    make(chan struct{}),
-		stoppedCh: make(chan struct{}),
-		logger:    logger,
+		config:          config,
+		stopCh:          make(chan struct{}),
+		stoppedCh:       make(chan struct{}),
+		logger:          logger,
+		commitPRCache:   make(map[string][]int),
+		commitCacheKeys: make([]string, 0, 512),
+		maxCacheSize:    512,
 	}, nil
 }
 
@@ -620,18 +631,119 @@ func (c *Client) readEvents(ctx context.Context, ws *websocket.Conn) error {
 			event.DeliveryID = deliveryID
 		}
 
+		if commitSHA, ok := response["commit_sha"].(string); ok {
+			event.CommitSHA = commitSHA
+		}
+
 		c.mu.Lock()
 		c.eventCount++
 		eventNum := c.eventCount
 		c.mu.Unlock()
 
+		// Handle check events with repo-only URLs (GitHub race condition)
+		// Automatically expand into per-PR events using GitHub API with caching
+		if (event.Type == "check_run" || event.Type == "check_suite") && event.CommitSHA != "" && !strings.Contains(event.URL, "/pull/") {
+			// Extract owner/repo from URL
+			parts := strings.Split(event.URL, "/")
+			if len(parts) >= 5 && parts[2] == "github.com" {
+				owner := parts[3]
+				repo := parts[4]
+				key := owner + "/" + repo + ":" + event.CommitSHA
+
+				// Check cache first
+				c.cacheMu.RLock()
+				cached, ok := c.commitPRCache[key]
+				c.cacheMu.RUnlock()
+
+				var prs []int
+				if ok {
+					// Cache hit - return copy to prevent external modifications
+					prs = make([]int, len(cached))
+					copy(prs, cached)
+					c.logger.Info("Check event with repo URL - using cached PR lookup",
+						"commit_sha", event.CommitSHA,
+						"repo_url", event.URL,
+						"type", event.Type,
+						"pr_count", len(prs),
+						"cache_hit", true)
+				} else {
+					// Cache miss - look up via GitHub API
+					c.logger.Info("Check event with repo URL - looking up PRs via GitHub API",
+						"commit_sha", event.CommitSHA,
+						"repo_url", event.URL,
+						"type", event.Type,
+						"cache_hit", false)
+
+					gh := github.NewClient(c.config.Token)
+					var err error
+					prs, err = gh.FindPRsForCommit(ctx, owner, repo, event.CommitSHA)
+					if err != nil {
+						c.logger.Warn("Failed to look up PRs for commit",
+							"commit_sha", event.CommitSHA,
+							"owner", owner,
+							"repo", repo,
+							"error", err)
+						// Don't cache errors - try again next time
+					} else {
+						// Cache the result (even if empty)
+						c.cacheMu.Lock()
+						if _, exists := c.commitPRCache[key]; !exists {
+							c.commitCacheKeys = append(c.commitCacheKeys, key)
+							// Evict oldest 25% if cache is full
+							if len(c.commitCacheKeys) > c.maxCacheSize {
+								n := c.maxCacheSize / 4
+								for i := range n {
+									delete(c.commitPRCache, c.commitCacheKeys[i])
+								}
+								c.commitCacheKeys = c.commitCacheKeys[n:]
+							}
+						}
+						// Store copy to prevent external modifications
+						cached := make([]int, len(prs))
+						copy(cached, prs)
+						c.commitPRCache[key] = cached
+						c.cacheMu.Unlock()
+
+						c.logger.Info("Cached PR lookup result",
+							"commit_sha", event.CommitSHA,
+							"pr_count", len(prs))
+					}
+				}
+
+				// Emit events for each PR found
+				if len(prs) > 0 {
+					for _, n := range prs {
+						e := event // Copy the event
+						e.URL = fmt.Sprintf("https://github.com/%s/%s/pull/%d", owner, repo, n)
+
+						if c.config.OnEvent != nil {
+							c.logger.Info("Event received (expanded from commit)",
+								"timestamp", e.Timestamp.Format("15:04:05"),
+								"event_number", eventNum,
+								"type", e.Type,
+								"url", e.URL,
+								"commit_sha", e.CommitSHA,
+								"delivery_id", e.DeliveryID)
+							c.config.OnEvent(e)
+						}
+					}
+					continue // Skip the normal event handling since we expanded it
+				}
+				c.logger.Info("No PRs found for commit - may be push to main",
+					"commit_sha", event.CommitSHA,
+					"owner", owner,
+					"repo", repo)
+			}
+		}
+
 		// Log event
 		if c.config.Verbose {
 			c.logger.Info("Event received",
 				"event_number", eventNum,
 				"timestamp", event.Timestamp.Format("15:04:05"),
 				"type", event.Type,
 				"url", event.URL,
+				"commit_sha", event.CommitSHA,
 				"delivery_id", event.DeliveryID,
 				"raw", event.Raw)
 		} else {
@@ -641,6 +753,7 @@ func (c *Client) readEvents(ctx context.Context, ws *websocket.Conn) error {
 					"event_number", eventNum,
 					"type", event.Type,
 					"url", event.URL,
+					"commit_sha", event.CommitSHA,
 					"delivery_id", event.DeliveryID)
 			} else {
 				c.logger.Info("Event received",

pkg/client/client_test.go

@@ -0,0 +1,79 @@
+package client
+
+import (
+	"context"
+	"sync"
+	"testing"
+	"time"
+)
+
+// TestStopMultipleCalls verifies that calling Stop() multiple times is safe
+// and doesn't panic with "close of closed channel".
+func TestStopMultipleCalls(t *testing.T) {
+	// Create a client with minimal config
+	client, err := New(Config{
+		ServerURL:    "ws://localhost:8080",
+		Token:        "test-token",
+		Organization: "test-org",
+		NoReconnect:  true, // Disable reconnect to make test faster
+	})
+	if err != nil {
+		t.Fatalf("Failed to create client: %v", err)
+	}
+
+	// Start the client in a goroutine
+	ctx, cancel := context.WithCancel(context.Background())
+	defer cancel()
+
+	go func() {
+		// Expected to fail to connect, but that's ok for this test
+		if err := client.Start(ctx); err != nil {
+			// Error is expected in tests - client can't connect to non-existent server
+		}
+	}()
+
+	// Give it a moment to initialize
+	time.Sleep(10 * time.Millisecond)
+
+	// Call Stop() multiple times concurrently
+	// This should NOT panic with "close of closed channel"
+	var wg sync.WaitGroup
+	for i := 0; i < 10; i++ {
+		wg.Add(1)
+		go func() {
+			defer wg.Done()
+			client.Stop() // Should be safe to call multiple times
+		}()
+	}
+
+	// Wait for all Stop() calls to complete
+	wg.Wait()
+
+	// If we get here without a panic, the test passes
+}
+
+// TestStopBeforeStart verifies that calling Stop() before Start() is safe.
+func TestStopBeforeStart(t *testing.T) {
+	client, err := New(Config{
+		ServerURL:    "ws://localhost:8080",
+		Token:        "test-token",
+		Organization: "test-org",
+		NoReconnect:  true,
+	})
+	if err != nil {
+		t.Fatalf("Failed to create client: %v", err)
+	}
+
+	// Call Stop() before Start()
+	client.Stop()
+
+	// Now try to start - should exit cleanly
+	ctx, cancel := context.WithTimeout(context.Background(), 100*time.Millisecond)
+	defer cancel()
+
+	err = client.Start(ctx)
+	// We expect either context.DeadlineExceeded or "stop requested"
+	if err == nil {
+		t.Error("Expected Start() to fail after Stop(), but it succeeded")
+	}
+}

pkg/github/client.go

@@ -463,3 +463,94 @@ func (c *Client) ValidateOrgMembership(ctx context.Context, org string) (usernam
 	log.Printf("GitHub API: User is member of %d organizations: %v", len(orgNames), orgNames)
 	return username, orgNames, errors.New("user is not a member of the requested organization")
 }
+
+// FindPRsForCommit finds all pull requests associated with a specific commit SHA.
+// This is useful for resolving check_run/check_suite events when GitHub's pull_requests array is empty.
+// Returns a list of PR numbers that contain this commit.
+func (c *Client) FindPRsForCommit(ctx context.Context, owner, repo, commitSHA string) ([]int, error) {
+	var prNumbers []int
+	var lastErr error
+
+	// Use GitHub's API to list PRs associated with a commit
+	url := fmt.Sprintf("https://api.github.com/repos/%s/%s/commits/%s/pulls", owner, repo, commitSHA)
+
+	err := retry.Do(
+		func() error {
+			req, err := http.NewRequestWithContext(ctx, http.MethodGet, url, http.NoBody)
+			if err != nil {
+				return fmt.Errorf("failed to create request: %w", err)
+			}
+
+			req.Header.Set("Authorization", fmt.Sprintf("Bearer %s", c.token))
+			req.Header.Set("Accept", "application/vnd.github.v3+json")
+			req.Header.Set("User-Agent", "webhook-sprinkler/1.0")
+
+			resp, err := c.httpClient.Do(req)
+			if err != nil {
+				lastErr = fmt.Errorf("failed to make request: %w", err)
+				log.Printf("GitHub API request failed (will retry): %v", err)
+				return err // Retry on network errors
+			}
+			defer func() {
+				if err := resp.Body.Close(); err != nil {
+					log.Printf("failed to close response body: %v", err)
+				}
+			}()
+
+			body, err := io.ReadAll(io.LimitReader(resp.Body, 1<<20)) // 1MB limit
+			if err != nil {
+				lastErr = fmt.Errorf("failed to read response: %w", err)
+				return err // Retry on read errors
+			}
+
+			// Handle status codes
+			switch resp.StatusCode {
+			case http.StatusOK:
+				// Success - parse response
+				var prs []struct {
+					Number int `json:"number"`
+				}
+				if err := json.Unmarshal(body, &prs); err != nil {
+					return retry.Unrecoverable(fmt.Errorf("failed to parse PR list response: %w", err))
+				}
+
+				prNumbers = make([]int, len(prs))
+				for i, pr := range prs {
+					prNumbers[i] = pr.Number
+				}
+				return nil
+
+			case http.StatusNotFound:
+				// Commit not found - could be a commit to main or repo doesn't exist
+				return retry.Unrecoverable(fmt.Errorf("commit not found: %s", commitSHA))
+
+			case http.StatusUnauthorized, http.StatusForbidden:
+				// Don't retry on auth errors
+				return retry.Unrecoverable(fmt.Errorf("authentication failed: status %d", resp.StatusCode))
+
+			case http.StatusInternalServerError, http.StatusBadGateway, http.StatusServiceUnavailable:
+				// Retry on server errors
+				lastErr = fmt.Errorf("GitHub API server error: %d", resp.StatusCode)
+				log.Printf("GitHub API: /commits/%s/pulls server error %d (will retry)", commitSHA, resp.StatusCode)
+				return lastErr
+
+			default:
+				// Don't retry on other errors
+				return retry.Unrecoverable(fmt.Errorf("unexpected status: %d, body: %s", resp.StatusCode, string(body)))
+			}
+		},
+		retry.Attempts(3),
+		retry.DelayType(retry.FullJitterBackoffDelay),
+		retry.MaxDelay(2*time.Minute),
+		retry.Context(ctx),
+	)
+	if err != nil {
+		if lastErr != nil {
+			return nil, lastErr
+		}
+		return nil, err
+	}
+
+	log.Printf("GitHub API: Found %d PR(s) for commit %s in %s/%s", len(prNumbers), commitSHA, owner, repo)
+	return prNumbers, nil
+}

pkg/srv/hub.go

@@ -14,10 +14,11 @@ import (
 // Event represents a GitHub webhook event that will be broadcast to clients.
 // It contains the PR URL, timestamp, event type, and delivery ID from GitHub.
 type Event struct {
-	URL        string    `json:"url"`                   // Pull request URL
-	Timestamp  time.Time `json:"timestamp"`             // When the event occurred
-	Type       string    `json:"type"`                  // GitHub event type (e.g., "pull_request")
-	DeliveryID string    `json:"delivery_id,omitempty"` // GitHub webhook delivery ID (unique per webhook)
+	URL        string    `json:"url"`                    // Pull request URL (or repo URL for check events with race condition)
+	Timestamp  time.Time `json:"timestamp"`              // When the event occurred
+	Type       string    `json:"type"`                   // GitHub event type (e.g., "pull_request")
+	DeliveryID string    `json:"delivery_id,omitempty"`  // GitHub webhook delivery ID (unique per webhook)
+	CommitSHA  string    `json:"commit_sha,omitempty"`   // Commit SHA for check events (used to look up PR when URL is repo-only)
 }
 
 // Hub manages WebSocket clients and event broadcasting.

pkg/webhook/handler.go

@@ -224,6 +224,11 @@ func (h *Handler) ServeHTTP(w http.ResponseWriter, r *http.Request) {
 		DeliveryID: deliveryID,
 	}
 
+	// For check events, include commit SHA to allow PR lookup when URL is repo-only (race condition)
+	if eventType == "check_run" || eventType == "check_suite" {
+		event.CommitSHA = extractCommitSHA(eventType, payload)
+	}
+
 	// Get client count before broadcasting (for debugging delivery issues)
 	clientCount := h.hub.ClientCount()