perf(github): cache check-run lookups with retry

Cache GitHub check-run API responses to avoid repeated
paginated API calls during status updates. Use a mutex+channel
pattern so concurrent goroutines share a single in-flight fetch.
Failed fetches are not cached, allowing retries with exponential
backoff on transient API errors.

Signed-off-by: Akshay Pant <akpant@redhat.com>
Assisted-by: Claude Opus 4.6 <noreply@anthropic.com>

Author: theakshaypant

pkg/provider/github/github.go

@@ -72,21 +72,31 @@ type Provider struct {
 	pacUserLogin       string // user/bot login used by PAC
 	clock              clockwork.Clock
 	graphQLClient      *graphQLClient
+	checkRunsCache     checkRunsCache
 }
 
 type skippedRun struct {
 	mutex      *sync.Mutex
 	checkRunID int64
 }
 
+type checkRunsCache struct {
+	mu        sync.Mutex
+	runs      []*github.CheckRun
+	loading   bool
+	done      chan struct{}
+	populated bool
+}
+
 func New() *Provider {
 	return &Provider{
 		APIURL:        github.Ptr(keys.PublicGithubAPIURL),
 		PaginedNumber: defaultPaginedNumber,
 		skippedRun: skippedRun{
 			mutex: &sync.Mutex{},
 		},
-		clock: clockwork.NewRealClock(),
+		clock:          clockwork.NewRealClock(),
+		checkRunsCache: checkRunsCache{},
 	}
 }
 

pkg/provider/github/status.go

@@ -23,8 +23,10 @@ import (
 )
 
 const (
-	botType         = "Bot"
-	pendingApproval = "Pending approval, waiting for an /ok-to-test"
+	botType                      = "Bot"
+	pendingApproval              = "Pending approval, waiting for an /ok-to-test"
+	checkRunsFetchMaxRetries     = 2
+	checkRunsFetchInitialBackoff = 200 * time.Millisecond
 )
 
 const taskStatusTemplate = `
@@ -42,7 +44,9 @@ const taskStatusTemplate = `
 {{- end }}
 </table>`
 
-func (v *Provider) getExistingCheckRunID(ctx context.Context, runevent *info.Event, status providerstatus.StatusOpts) (*int64, error) {
+// fetchAllCheckRunPages retrieves every page of check runs for the event SHA.
+func (v *Provider) fetchAllCheckRunPages(ctx context.Context, runevent *info.Event) ([]*github.CheckRun, error) {
+	var all []*github.CheckRun
 	opt := github.ListOptions{PerPage: v.PaginedNumber}
 	for {
 		res, resp, err := wrapAPI(v, "list_check_runs_for_ref", func() (*github.ListCheckRunsResults, *github.Response, error) {
@@ -55,25 +59,95 @@ func (v *Provider) getExistingCheckRunID(ctx context.Context, runevent *info.Eve
 		if err != nil {
 			return nil, err
 		}
-
-		for _, checkrun := range res.CheckRuns {
-			// if it is a Pending approval CheckRun then overwrite it
-			if isPendingApprovalCheckrun(checkrun) || isFailedCheckrun(checkrun) {
-				if v.canIUseCheckrunID(checkrun.ID) {
-					return checkrun.ID, nil
-				}
-			}
-			if *checkrun.ExternalID == status.PipelineRunName {
-				return checkrun.ID, nil
-			}
-		}
+		all = append(all, res.CheckRuns...)
 		if resp.NextPage == 0 {
 			break
 		}
 		opt.Page = resp.NextPage
 	}
+	return all, nil
+}
+
+func (v *Provider) searchCheckRuns(runs []*github.CheckRun, status providerstatus.StatusOpts) *int64 {
+	for _, checkrun := range runs {
+		if isPendingApprovalCheckrun(checkrun) || isFailedCheckrun(checkrun) {
+			if v.canIUseCheckrunID(checkrun.ID) {
+				return checkrun.ID
+			}
+		}
+		if checkrun.ExternalID != nil && *checkrun.ExternalID == status.PipelineRunName {
+			return checkrun.ID
+		}
+	}
+	return nil
+}
+
+func (v *Provider) getExistingCheckRunID(ctx context.Context, runevent *info.Event, status providerstatus.StatusOpts) (*int64, error) {
+	for {
+		v.checkRunsCache.mu.Lock()
+		if v.checkRunsCache.loading {
+			done := v.checkRunsCache.done
+			v.checkRunsCache.mu.Unlock()
+			<-done
+			continue
+		}
+		if v.checkRunsCache.populated {
+			runs := v.checkRunsCache.runs
+			v.checkRunsCache.mu.Unlock()
+			return v.searchCheckRuns(runs, status), nil
+		}
+
+		v.checkRunsCache.loading = true
+		v.checkRunsCache.done = make(chan struct{})
+		v.checkRunsCache.mu.Unlock()
+
+		runs, err := v.fetchAllCheckRunPagesWithRetry(ctx, runevent)
+
+		v.checkRunsCache.mu.Lock()
+		if err != nil {
+			v.checkRunsCache.loading = false
+		} else {
+			v.checkRunsCache.runs = runs
+			v.checkRunsCache.loading = false
+			v.checkRunsCache.populated = true
+		}
+		close(v.checkRunsCache.done)
+		v.checkRunsCache.mu.Unlock()
+
+		if err != nil {
+			return nil, err
+		}
+		return v.searchCheckRuns(runs, status), nil
+	}
+}
+
+func (v *Provider) fetchAllCheckRunPagesWithRetry(ctx context.Context, runevent *info.Event) ([]*github.CheckRun, error) {
+	var lastErr error
+	for attempt := range checkRunsFetchMaxRetries + 1 {
+		runs, err := v.fetchAllCheckRunPages(ctx, runevent)
+		if err == nil {
+			return runs, nil
+		}
+		lastErr = err
+
+		if attempt == checkRunsFetchMaxRetries {
+			return nil, err
+		}
+
+		backoff := time.Duration(1<<uint(attempt)) * checkRunsFetchInitialBackoff
+		if v.Logger != nil {
+			v.Logger.Debugf("check-runs lookup failed for %s/%s@%s (attempt %d/%d): %v; retrying in %v",
+				runevent.Organization, runevent.Repository, runevent.SHA,
+				attempt+1, checkRunsFetchMaxRetries+1, err, backoff)
+		}
 
-	return nil, nil
+		select {
+		case <-ctx.Done():
+			return nil, ctx.Err()
+		case <-v.getClock().After(backoff):
+		}
+	}
+	return nil, lastErr
 }
 
 func isPendingApprovalCheckrun(run *github.CheckRun) bool {

pkg/provider/github/status_test.go

@@ -8,9 +8,13 @@ import (
 	"reflect"
 	"strconv"
 	"strings"
+	"sync"
+	"sync/atomic"
 	"testing"
+	"time"
 
 	"github.com/google/go-github/v84/github"
+	"github.com/jonboulle/clockwork"
 	"github.com/openshift-pipelines/pipelines-as-code/pkg/apis/pipelinesascode/keys"
 	"github.com/openshift-pipelines/pipelines-as-code/pkg/params"
 	"github.com/openshift-pipelines/pipelines-as-code/pkg/params/clients"
@@ -686,3 +690,117 @@ func TestProviderGetExistingCheckRunID(t *testing.T) {
 		})
 	}
 }
+
+func TestGetExistingCheckRunIDCacheHit(t *testing.T) {
+	ctx, _ := rtesting.SetupFakeContext(t)
+	client, mux, _, teardown := ghtesthelper.SetupGH()
+	defer teardown()
+
+	event := &info.Event{
+		Organization: "owner",
+		Repository:   "repository",
+		SHA:          "sha",
+	}
+
+	apiHits := 0
+	mux.HandleFunc(fmt.Sprintf("/repos/%v/%v/commits/%v/check-runs", event.Organization, event.Repository, event.SHA), func(w http.ResponseWriter, _ *http.Request) {
+		apiHits++
+		fmt.Fprint(w, `{"total_count": 2, "check_runs": [{"id": 55555, "external_id": "mypr"}, {"id": 55556, "external_id": "mypr2"}]}`)
+	})
+
+	cnx := New()
+	cnx.SetGithubClient(client)
+
+	// First call should hit the API and populate the cache.
+	id, err := cnx.getExistingCheckRunID(ctx, event, providerstatus.StatusOpts{PipelineRunName: "mypr"})
+	assert.NilError(t, err)
+	assert.Assert(t, id != nil)
+
+	// Second call should serve from cache.
+	id2, err := cnx.getExistingCheckRunID(ctx, event, providerstatus.StatusOpts{PipelineRunName: "mypr2"})
+	assert.NilError(t, err)
+	assert.Assert(t, id2 != nil)
+	assert.Equal(t, apiHits, 1)
+}
+
+func TestGetExistingCheckRunIDConcurrent(t *testing.T) {
+	ctx, _ := rtesting.SetupFakeContext(t)
+	client, mux, _, teardown := ghtesthelper.SetupGH()
+	defer teardown()
+
+	event := &info.Event{
+		Organization: "owner",
+		Repository:   "repository",
+		SHA:          "sha",
+	}
+
+	var apiHits atomic.Int64
+	mux.HandleFunc(fmt.Sprintf("/repos/%v/%v/commits/%v/check-runs", event.Organization, event.Repository, event.SHA), func(w http.ResponseWriter, _ *http.Request) {
+		apiHits.Add(1)
+		fmt.Fprint(w, `{"total_count": 2, "check_runs": [{"id": 55555, "external_id": "mypr"}, {"id": 55556, "external_id": "mypr2"}]}`)
+	})
+
+	cnx := New()
+	cnx.SetGithubClient(client)
+
+	const goroutines = 10
+	var wg sync.WaitGroup
+	wg.Add(goroutines)
+	for range goroutines {
+		go func() {
+			defer wg.Done()
+			_, _ = cnx.getExistingCheckRunID(ctx, event, providerstatus.StatusOpts{PipelineRunName: "mypr"})
+		}()
+	}
+	wg.Wait()
+
+	// Only one goroutine should have fetched from the API; the rest hit the cache.
+	assert.Equal(t, apiHits.Load(), int64(1))
+}
+
+func TestGetExistingCheckRunIDRetryOnError(t *testing.T) {
+	ctx, _ := rtesting.SetupFakeContext(t)
+	client, mux, _, teardown := ghtesthelper.SetupGH()
+	defer teardown()
+
+	event := &info.Event{
+		Organization: "owner",
+		Repository:   "repository",
+		SHA:          "sha",
+	}
+
+	var apiHits atomic.Int64
+	mux.HandleFunc(fmt.Sprintf("/repos/%v/%v/commits/%v/check-runs", event.Organization, event.Repository, event.SHA), func(w http.ResponseWriter, _ *http.Request) {
+		hit := apiHits.Add(1)
+		if hit == 1 {
+			// First call: simulate a transient GitHub 502.
+			w.WriteHeader(http.StatusBadGateway)
+			return
+		}
+		// Second call: succeed.
+		fmt.Fprint(w, `{"total_count": 1, "check_runs": [{"id": 77777, "external_id": "mypr"}]}`)
+	})
+
+	cnx := New()
+	cnx.SetGithubClient(client)
+	clock := clockwork.NewFakeClock()
+	cnx.clock = clock
+
+	errCh := make(chan error, 1)
+	var id *int64
+	go func() {
+		var err error
+		id, err = cnx.getExistingCheckRunID(ctx, event, providerstatus.StatusOpts{PipelineRunName: "mypr"})
+		errCh <- err
+	}()
+
+	// Wait for the first attempt to fail and backoff timer to start
+	_ = clock.BlockUntilContext(ctx, 1)
+	clock.Advance(200 * time.Millisecond)
+
+	err := <-errCh
+	assert.NilError(t, err)
+	assert.Assert(t, id != nil)
+	assert.Equal(t, *id, int64(77777))
+	assert.Equal(t, apiHits.Load(), int64(2))
+}