Skip to content

Commit ec25791

Browse files
authored
Merge pull request #43 from codeGROOVE-dev/reliable
Auto-refresh GitHub tokens
2 parents 85c5be8 + d5c4857 commit ec25791

File tree

3 files changed

+57
-14
lines changed

3 files changed

+57
-14
lines changed

internal/bot/bot_sprinkler.go

Lines changed: 18 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -318,12 +318,24 @@ func (c *Coordinator) RunWithSprinklerClient(ctx context.Context) error {
318318
if startErr != nil {
319319
errCount++
320320

321-
// Check if it's an authentication error
322-
if strings.Contains(startErr.Error(), "403") || strings.Contains(startErr.Error(), "401") {
323-
slog.Warn("authentication failed, refreshing token",
324-
"organization", organization,
325-
"consecutive_errors", errCount,
326-
"error", startErr)
321+
// Check if it's an authentication error OR if we've had many failures (token might be expired)
322+
// After 5 consecutive failures, proactively refresh the token
323+
isAuthError := strings.Contains(startErr.Error(), "403") || strings.Contains(startErr.Error(), "401")
324+
shouldRefreshToken := isAuthError || errCount >= 5
325+
326+
if shouldRefreshToken {
327+
if isAuthError {
328+
slog.Warn("authentication failed, refreshing token",
329+
"organization", organization,
330+
"consecutive_errors", errCount,
331+
"error", startErr)
332+
} else {
333+
slog.Info("multiple connection failures, proactively refreshing token",
334+
"organization", organization,
335+
"consecutive_errors", errCount,
336+
"error", startErr,
337+
"reason", "token may have expired")
338+
}
327339

328340
sprinklerClient.Stop() // Stop old client before creating new one
329341

internal/config/config.go

Lines changed: 17 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -7,6 +7,7 @@ import (
77
"fmt"
88
"log/slog"
99
"net/http"
10+
"strings"
1011
"sync"
1112
"time"
1213

@@ -427,6 +428,9 @@ func (m *Manager) ChannelsForRepo(org, repo string) []string {
427428

428429
// First, check explicit YAML configuration
429430
for channelName, channelConfig := range config.Channels {
431+
// Normalize channel name to lowercase (Slack channels are always lowercase)
432+
normalizedChannelName := strings.ToLower(channelName)
433+
430434
// Check if this channel explicitly includes this repo
431435
for _, configRepo := range channelConfig.Repos {
432436
if m.matchesRepo(configRepo, repo) {
@@ -436,11 +440,11 @@ func (m *Manager) ChannelsForRepo(org, repo string) []string {
436440
slog.Debug("skipping explicitly muted channel",
437441
logFieldOrg, org,
438442
"repo", repo,
439-
"channel", channelName,
443+
"channel", normalizedChannelName,
440444
"muted", true)
441445
continue
442446
}
443-
channels = append(channels, channelName)
447+
channels = append(channels, normalizedChannelName)
444448
break // Don't add the same channel multiple times
445449
}
446450
}
@@ -467,7 +471,15 @@ func (m *Manager) ChannelsForRepo(org, repo string) []string {
467471
}
468472

469473
// Check if auto-discovered channel is explicitly muted
470-
if channelConfig, exists := config.Channels[autoChannel]; exists && channelConfig.Mute {
474+
// Need to check case-insensitively since YAML keys preserve case
475+
var channelMuted bool
476+
for yamlChannelName, channelConfig := range config.Channels {
477+
if strings.EqualFold(yamlChannelName, autoChannel) && channelConfig.Mute {
478+
channelMuted = true
479+
break
480+
}
481+
}
482+
if channelMuted {
471483
slog.Info("auto-discovered channel is explicitly muted in config",
472484
logFieldOrg, org,
473485
"repo", repo,
@@ -518,8 +530,8 @@ func (*Manager) matchesRepo(pattern, repo string) bool {
518530
// For example: repo "goose" -> channel "#goose", repo "my-service" -> channel "#my-service".
519531
func (*Manager) autoDiscoverChannels(org, repo string) []string {
520532
// Convert repo name to channel name
521-
// Most repos will match their channel name directly
522-
channelName := repo
533+
// Slack channel names are always lowercase
534+
channelName := strings.ToLower(repo)
523535

524536
slog.Debug("attempting auto-discovery of channel for repo",
525537
logFieldOrg, org,

internal/github/github.go

Lines changed: 22 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -37,6 +37,22 @@ type Client struct {
3737
tokenMutex sync.RWMutex
3838
}
3939

40+
// refreshingTokenSource implements oauth2.TokenSource that automatically refreshes tokens.
41+
type refreshingTokenSource struct {
42+
client *Client
43+
}
44+
45+
// Token returns a fresh token, refreshing if necessary.
46+
func (ts *refreshingTokenSource) Token() (*oauth2.Token, error) {
47+
// Use a background context for token refresh - token operations should complete
48+
// independently of request contexts to avoid breaking long-running connections
49+
token := ts.client.InstallationToken(context.Background())
50+
if token == "" {
51+
return nil, errors.New("no token available")
52+
}
53+
return &oauth2.Token{AccessToken: token}, nil
54+
}
55+
4056
// userAgentTransport adds a custom User-Agent header to requests.
4157
type userAgentTransport struct {
4258
base http.RoundTripper
@@ -186,16 +202,19 @@ func (c *Client) authenticate(ctx context.Context) error {
186202
"installation_id", c.installationID)
187203
}
188204

189-
// Create installation client with custom user-agent.
190-
ts = oauth2.StaticTokenSource(&oauth2.Token{AccessToken: token.GetToken()})
205+
// Create installation client with auto-refreshing token source and custom user-agent.
206+
// The refreshingTokenSource will automatically call InstallationToken() which handles
207+
// token expiry checking and refreshing.
208+
ts = &refreshingTokenSource{client: c}
191209
tc = oauth2.NewClient(ctx, ts)
192210
tc.Transport = &userAgentTransport{base: tc.Transport}
193211
c.client = github.NewClient(tc)
194212

195213
// Store the token with expiry (GitHub tokens expire after 1 hour).
214+
// For security, refresh every 30 minutes instead of waiting until near expiry.
196215
c.tokenMutex.Lock()
197216
c.installationToken = token.GetToken()
198-
c.tokenExpiry = time.Now().Add(55 * time.Minute) // Refresh 5 minutes before expiry
217+
c.tokenExpiry = time.Now().Add(30 * time.Minute) // Refresh every 30 minutes for security
199218
c.tokenMutex.Unlock()
200219

201220
// Test the token by making a simple API call

0 commit comments

Comments
 (0)