Skip to content

Commit 9e5be0b

Browse files
tstrombergtstromberg
authored
Merge pull request #68 from codeGROOVE-dev/reliable
add turnclient as a commit->PR lookup fallback
2 parents c965c11 + 51a250b commit 9e5be0b

File tree

4 files changed

+789
-4
lines changed

4 files changed

+789
-4
lines changed

‎pkg/bot/bot.go‎

Lines changed: 145 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -51,6 +51,21 @@ type ThreadCache struct {
5151
// ThreadInfo is an alias to state.ThreadInfo to avoid duplication.
5252
type ThreadInfo = state.ThreadInfo
5353

54+
// CommitPREntry caches recent commit→PR mappings for fast lookup.
55+
type CommitPREntry struct {
56+
PRNumber int
57+
HeadSHA string
58+
UpdatedAt time.Time
59+
}
60+
61+
// CommitPRCache provides in-memory caching of commit SHA → PR mappings.
62+
// This allows quick lookup when check events arrive with just a commit SHA,
63+
// avoiding expensive GitHub API calls for recently-seen PRs.
64+
type CommitPRCache struct {
65+
mu sync.RWMutex
66+
entries map[string][]CommitPREntry // "owner/repo" -> recent PRs with commits
67+
}
68+
5469
// Get retrieves thread info for a PR.
5570
func (tc *ThreadCache) Get(prKey string) (ThreadInfo, bool) {
5671
tc.mu.RLock()
@@ -81,6 +96,113 @@ func (tc *ThreadCache) Cleanup(maxAge time.Duration) {
8196
}
8297
}
8398

99+
// RecordPR records a PR's head commit SHA for commit→PR lookups.
100+
// Entries are kept for 10 minutes to handle check events that arrive shortly after PR events.
101+
func (cpc *CommitPRCache) RecordPR(owner, repo string, prNumber int, headSHA string) {
102+
if headSHA == "" {
103+
return // Skip empty commits
104+
}
105+
106+
cpc.mu.Lock()
107+
defer cpc.mu.Unlock()
108+
109+
repoKey := owner + "/" + repo
110+
now := time.Now()
111+
112+
// Initialize map if needed
113+
if cpc.entries == nil {
114+
cpc.entries = make(map[string][]CommitPREntry)
115+
}
116+
117+
// Add new entry
118+
entry := CommitPREntry{
119+
PRNumber: prNumber,
120+
HeadSHA: headSHA,
121+
UpdatedAt: now,
122+
}
123+
124+
// Get existing entries for this repo
125+
entries := cpc.entries[repoKey]
126+
127+
// Check if this exact PR+commit combination already exists - update timestamp if so
128+
found := false
129+
for i := range entries {
130+
if entries[i].PRNumber == prNumber && entries[i].HeadSHA == headSHA {
131+
entries[i].UpdatedAt = now // Refresh timestamp
132+
found = true
133+
break
134+
}
135+
}
136+
137+
if !found {
138+
entries = append(entries, entry)
139+
}
140+
141+
// Update the map with the modified entries before filtering
142+
cpc.entries[repoKey] = entries
143+
144+
// Keep only entries from last 10 minutes (check events usually arrive within seconds)
145+
cutoff := now.Add(-10 * time.Minute)
146+
filtered := make([]CommitPREntry, 0, len(entries))
147+
for i := range entries {
148+
if entries[i].UpdatedAt.After(cutoff) {
149+
filtered = append(filtered, entries[i])
150+
}
151+
}
152+
153+
cpc.entries[repoKey] = filtered
154+
}
155+
156+
// FindPRsForCommit finds PRs in a repo that match the given commit SHA.
157+
// Returns PR numbers if found in recent cache (last 10 minutes), nil otherwise.
158+
func (cpc *CommitPRCache) FindPRsForCommit(owner, repo, commitSHA string) []int {
159+
if commitSHA == "" {
160+
return nil
161+
}
162+
163+
cpc.mu.RLock()
164+
defer cpc.mu.RUnlock()
165+
166+
repoKey := owner + "/" + repo
167+
entries, exists := cpc.entries[repoKey]
168+
if !exists {
169+
return nil
170+
}
171+
172+
// Check which PRs have this commit
173+
var prNumbers []int
174+
for i := range entries {
175+
if entries[i].HeadSHA == commitSHA {
176+
prNumbers = append(prNumbers, entries[i].PRNumber)
177+
}
178+
}
179+
180+
return prNumbers
181+
}
182+
183+
// MostRecentPR returns the most recently updated PR number for a repo from the cache.
184+
// Returns 0 if no recent PRs are cached for this repo.
185+
func (cpc *CommitPRCache) MostRecentPR(owner, repo string) int {
186+
cpc.mu.RLock()
187+
defer cpc.mu.RUnlock()
188+
189+
repoKey := owner + "/" + repo
190+
entries, exists := cpc.entries[repoKey]
191+
if !exists || len(entries) == 0 {
192+
return 0
193+
}
194+
195+
// Find the entry with the most recent UpdatedAt timestamp
196+
mostRecent := entries[0]
197+
for i := 1; i < len(entries); i++ {
198+
if entries[i].UpdatedAt.After(mostRecent.UpdatedAt) {
199+
mostRecent = entries[i]
200+
}
201+
}
202+
203+
return mostRecent.PRNumber
204+
}
205+
84206
// Coordinator coordinates between GitHub, Slack, and notifications for a single org.
85207
//
86208
//nolint:govet // Field order optimized for logical grouping over memory alignment
@@ -94,8 +216,9 @@ type Coordinator struct {
94216
configManager *config.Manager
95217
notifier *notify.Manager
96218
userMapper *usermapping.Service
97-
threadCache *ThreadCache // In-memory cache for fast lookups
98-
eventSemaphore chan struct{} // Limits concurrent event processing (prevents overwhelming APIs)
219+
threadCache *ThreadCache // In-memory cache for fast lookups
220+
commitPRCache *CommitPRCache // Maps commit SHAs to PR numbers for check events
221+
eventSemaphore chan struct{} // Limits concurrent event processing (prevents overwhelming APIs)
99222
}
100223

101224
// StateStore interface for persistent state - allows dependency injection for testing.
@@ -134,6 +257,9 @@ func New(
134257
prThreads: make(map[string]ThreadInfo),
135258
creating: make(map[string]bool),
136259
},
260+
commitPRCache: &CommitPRCache{
261+
entries: make(map[string][]CommitPREntry),
262+
},
137263
eventSemaphore: make(chan struct{}, 10), // Allow 10 concurrent events per org
138264
}
139265

@@ -1546,6 +1672,23 @@ func (c *Coordinator) handlePullRequestFromSprinkler(
15461672
// Use PR details from turnclient instead of making additional GitHub API call
15471673
pr := checkResult.PullRequest
15481674

1675+
// Populate commit→PR cache for all commits in this PR
1676+
// This allows us to quickly map check events (which only have commit SHA) back to PRs
1677+
// without making expensive GitHub API calls
1678+
if len(pr.Commits) > 0 {
1679+
slog.Debug("populating commit→PR cache from turnclient response",
1680+
logFieldOwner, owner,
1681+
logFieldRepo, repo,
1682+
"pr_number", prNumber,
1683+
"commit_count", len(pr.Commits))
1684+
1685+
for _, commitSHA := range pr.Commits {
1686+
if commitSHA != "" {
1687+
c.commitPRCache.RecordPR(owner, repo, prNumber, commitSHA)
1688+
}
1689+
}
1690+
}
1691+
15491692
// Create a synthetic webhook event to reuse existing logic with real PR data
15501693
event := struct {
15511694
Action string `json:"action"`

‎pkg/bot/bot_sprinkler.go‎

Lines changed: 132 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -11,6 +11,7 @@ import (
1111

1212
"github.com/codeGROOVE-dev/slacker/pkg/state"
1313
"github.com/codeGROOVE-dev/sprinkler/pkg/client"
14+
"github.com/codeGROOVE-dev/turnclient/pkg/turn"
1415
)
1516

1617
// Constants for URL parsing.
@@ -84,13 +85,142 @@ func (c *Coordinator) lookupPRsForCheckEvent(ctx context.Context, event client.E
8485
owner := parts[3]
8586
repo := parts[4]
8687

87-
slog.Info("sprinkler cache miss - looking up PRs for commit via GitHub API",
88+
// First, check our local commit→PR cache (populated from recent PR events)
89+
// This is MUCH faster than GitHub API and usually works since check events
90+
// arrive seconds after PR events
91+
cachedPRs := c.commitPRCache.FindPRsForCommit(owner, repo, commitSHA)
92+
if len(cachedPRs) > 0 {
93+
slog.Info("found PRs for commit in cache - avoiding GitHub API call",
94+
"organization", organization,
95+
"owner", owner,
96+
"repo", repo,
97+
"commit_sha", commitSHA,
98+
"pr_count", len(cachedPRs),
99+
"pr_numbers", cachedPRs,
100+
"type", event.Type,
101+
"delivery_id", deliveryID,
102+
"cache_hit", true)
103+
return cachedPRs
104+
}
105+
106+
// Cache miss - try turnclient lookup on most recent PR before falling back to GitHub API
107+
slog.Info("commit→PR cache miss - will try turnclient on recent PR before GitHub API",
88108
"organization", organization,
89109
"owner", owner,
90110
"repo", repo,
91111
"commit_sha", commitSHA,
92112
"type", event.Type,
93-
"delivery_id", deliveryID)
113+
"delivery_id", deliveryID,
114+
"cache_hit", false,
115+
"reason", "check event arrived before PR event or cache expired")
116+
117+
// Second attempt: Check if we recently saw a PR for this repo
118+
// If yes, fetch it via turnclient to see if it contains this commit
119+
// This is cheaper than searching all PRs via GitHub API
120+
mostRecentPR := c.commitPRCache.MostRecentPR(owner, repo)
121+
if mostRecentPR > 0 {
122+
slog.Debug("attempting turnclient lookup on most recent PR for repo",
123+
"organization", organization,
124+
"owner", owner,
125+
"repo", repo,
126+
"pr_number", mostRecentPR,
127+
"commit_sha", commitSHA,
128+
"rationale", "check events usually arrive for recently updated PRs")
129+
130+
// Get GitHub token
131+
githubToken := c.github.InstallationToken(ctx)
132+
if githubToken != "" {
133+
// Create turnclient
134+
turnClient, tcErr := turn.NewDefaultClient()
135+
if tcErr == nil {
136+
turnClient.SetAuthToken(githubToken)
137+
138+
// Check the recent PR with current timestamp
139+
checkCtx, checkCancel := context.WithTimeout(ctx, 30*time.Second)
140+
prURL := fmt.Sprintf("https://github.com/%s/%s/pull/%d", owner, repo, mostRecentPR)
141+
checkResult, checkErr := turnClient.Check(checkCtx, prURL, owner, time.Now())
142+
checkCancel()
143+
144+
if checkErr == nil && checkResult != nil {
145+
// Check if any commits match
146+
for _, prCommit := range checkResult.PullRequest.Commits {
147+
if prCommit == commitSHA {
148+
slog.Info("found commit in most recent PR via turnclient - avoiding GitHub API search",
149+
"organization", organization,
150+
"owner", owner,
151+
"repo", repo,
152+
"pr_number", mostRecentPR,
153+
"commit_sha", commitSHA,
154+
"turnclient_hit", true,
155+
"bonus", "got free PR status update")
156+
157+
// Populate cache with all commits from this PR
158+
for _, commit := range checkResult.PullRequest.Commits {
159+
if commit != "" {
160+
c.commitPRCache.RecordPR(owner, repo, mostRecentPR, commit)
161+
}
162+
}
163+
164+
// Process the PR update since we have fresh data
165+
go c.handlePullRequestEventWithData(context.Background(), owner, repo, struct {
166+
Action string `json:"action"`
167+
PullRequest struct {
168+
HTMLURL string `json:"html_url"`
169+
Title string `json:"title"`
170+
CreatedAt time.Time `json:"created_at"`
171+
User struct {
172+
Login string `json:"login"`
173+
} `json:"user"`
174+
Number int `json:"number"`
175+
} `json:"pull_request"`
176+
Number int `json:"number"`
177+
}{
178+
Action: "synchronize",
179+
PullRequest: struct {
180+
HTMLURL string `json:"html_url"`
181+
Title string `json:"title"`
182+
CreatedAt time.Time `json:"created_at"`
183+
User struct {
184+
Login string `json:"login"`
185+
} `json:"user"`
186+
Number int `json:"number"`
187+
}{
188+
HTMLURL: prURL,
189+
Title: checkResult.PullRequest.Title,
190+
CreatedAt: checkResult.PullRequest.CreatedAt,
191+
User: struct {
192+
Login string `json:"login"`
193+
}{
194+
Login: checkResult.PullRequest.Author,
195+
},
196+
Number: mostRecentPR,
197+
},
198+
Number: mostRecentPR,
199+
}, checkResult, nil)
200+
201+
return []int{mostRecentPR}
202+
}
203+
}
204+
205+
slog.Debug("commit not found in most recent PR - will fall back to GitHub API",
206+
"organization", organization,
207+
"owner", owner,
208+
"repo", repo,
209+
"pr_number", mostRecentPR,
210+
"commit_sha", commitSHA,
211+
"pr_commits_checked", len(checkResult.PullRequest.Commits))
212+
}
213+
}
214+
}
215+
}
216+
217+
// Third attempt: Fall back to GitHub API to search all PRs
218+
slog.Info("falling back to GitHub API to search all PRs for commit",
219+
"organization", organization,
220+
"owner", owner,
221+
"repo", repo,
222+
"commit_sha", commitSHA,
223+
"tried_turnclient", mostRecentPR > 0)
94224

95225
// Look up PRs for this commit using GitHub API
96226
// Allow up to 3 minutes for retries (2 min max delay + buffer)

0 commit comments

Comments
 (0)