Skip to content

Commit b1a5631

Browse files
authored
Merge pull request #45 from codeGROOVE-dev/reliable
Include today in closed PR check, lint
2 parents 87be1a3 + f070ef4 commit b1a5631

File tree

13 files changed

+159
-94
lines changed

13 files changed

+159
-94
lines changed

cmd/server/main.go

Lines changed: 8 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -161,15 +161,15 @@ func run(ctx context.Context, cancel context.CancelFunc, cfg *config.ServerConfi
161161

162162
// Initialize state store (Datastore + JSON fallback).
163163
var stateStore interface {
164-
GetThread(owner, repo string, number int, channelID string) (state.ThreadInfo, bool)
164+
Thread(owner, repo string, number int, channelID string) (state.ThreadInfo, bool)
165165
SaveThread(owner, repo string, number int, channelID string, info state.ThreadInfo) error
166-
GetLastDM(userID, prURL string) (time.Time, bool)
166+
LastDM(userID, prURL string) (time.Time, bool)
167167
RecordDM(userID, prURL string, sentAt time.Time) error
168-
GetLastDigest(userID, date string) (time.Time, bool)
168+
LastDigest(userID, date string) (time.Time, bool)
169169
RecordDigest(userID, date string, sentAt time.Time) error
170170
WasProcessed(eventKey string) bool
171171
MarkProcessed(eventKey string, ttl time.Duration) error
172-
GetLastNotification(prURL string) time.Time
172+
LastNotification(prURL string) time.Time
173173
RecordNotification(prURL string, notifiedAt time.Time) error
174174
Cleanup() error
175175
Close() error
@@ -668,15 +668,15 @@ func runBotCoordinators(
668668
configManager *config.Manager,
669669
notifier *notify.Manager,
670670
stateStore interface {
671-
GetThread(owner, repo string, number int, channelID string) (state.ThreadInfo, bool)
671+
Thread(owner, repo string, number int, channelID string) (state.ThreadInfo, bool)
672672
SaveThread(owner, repo string, number int, channelID string, info state.ThreadInfo) error
673-
GetLastDM(userID, prURL string) (time.Time, bool)
673+
LastDM(userID, prURL string) (time.Time, bool)
674674
RecordDM(userID, prURL string, sentAt time.Time) error
675-
GetLastDigest(userID, date string) (time.Time, bool)
675+
LastDigest(userID, date string) (time.Time, bool)
676676
RecordDigest(userID, date string, sentAt time.Time) error
677677
WasProcessed(eventKey string) bool
678678
MarkProcessed(eventKey string, ttl time.Duration) error
679-
GetLastNotification(prURL string) time.Time
679+
LastNotification(prURL string) time.Time
680680
RecordNotification(prURL string, notifiedAt time.Time) error
681681
Cleanup() error
682682
Close() error

go.mod

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -7,7 +7,7 @@ require (
77
github.com/codeGROOVE-dev/gh-mailto v0.0.0-20251019162917-c3412c017b1f
88
github.com/codeGROOVE-dev/gsm v0.0.0-20251019065141-833fe2363d22
99
github.com/codeGROOVE-dev/retry v1.2.0
10-
github.com/codeGROOVE-dev/sprinkler v0.0.0-20251020064313-f606185b6b98
10+
github.com/codeGROOVE-dev/sprinkler v0.0.0-20251020140418-efb533e2ff51
1111
github.com/codeGROOVE-dev/turnclient v0.0.0-20251018202306-7cdc0d51856e
1212
github.com/golang-jwt/jwt/v5 v5.3.0
1313
github.com/google/go-github/v50 v50.2.0

go.sum

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -22,8 +22,8 @@ github.com/codeGROOVE-dev/prx v0.0.0-20251016165946-00c6c6e90c29 h1:MSBy3Ywr3ky/
2222
github.com/codeGROOVE-dev/prx v0.0.0-20251016165946-00c6c6e90c29/go.mod h1:7qLbi18baOyS8yO/6/64SBIqtyzSzLFdsDST15NPH3w=
2323
github.com/codeGROOVE-dev/retry v1.2.0 h1:xYpYPX2PQZmdHwuiQAGGzsBm392xIMl4nfMEFApQnu8=
2424
github.com/codeGROOVE-dev/retry v1.2.0/go.mod h1:8OgefgV1XP7lzX2PdKlCXILsYKuz6b4ZpHa/20iLi8E=
25-
github.com/codeGROOVE-dev/sprinkler v0.0.0-20251020064313-f606185b6b98 h1:unjiIF1rx/QZfcTEW/n6EJjde1yd3b1ZbjrWee2Afj4=
26-
github.com/codeGROOVE-dev/sprinkler v0.0.0-20251020064313-f606185b6b98/go.mod h1:/kd3ncsRNldD0MUpbtp5ojIzfCkyeXB7JdOrpuqG7Gg=
25+
github.com/codeGROOVE-dev/sprinkler v0.0.0-20251020140418-efb533e2ff51 h1:oPVbUoZ1jxgmrqybgRCfhwdT8KaXE/hzQ4vAswRybt0=
26+
github.com/codeGROOVE-dev/sprinkler v0.0.0-20251020140418-efb533e2ff51/go.mod h1:/kd3ncsRNldD0MUpbtp5ojIzfCkyeXB7JdOrpuqG7Gg=
2727
github.com/codeGROOVE-dev/turnclient v0.0.0-20251018202306-7cdc0d51856e h1:3qoY6h8SgoeNsIYRM7P6PegTXAHPo8OSOapUunVP/Gs=
2828
github.com/codeGROOVE-dev/turnclient v0.0.0-20251018202306-7cdc0d51856e/go.mod h1:fYwtN9Ql6lY8t2WvCfENx+mP5FUwjlqwXCLx9CVLY20=
2929
github.com/davecgh/go-spew v1.1.1 h1:vj9j/u1bqnvCEfJOwUhtlOARqs3+rkHYY13jYWTU97c=

internal/bot/bot.go

Lines changed: 17 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -83,27 +83,28 @@ func (tc *ThreadCache) Cleanup(maxAge time.Duration) {
8383

8484
// Coordinator coordinates between GitHub, Slack, and notifications for a single org.
8585
type Coordinator struct {
86-
slack *slackpkg.Client
87-
github *github.Client
88-
configManager *config.Manager
89-
notifier *notify.Manager
90-
userMapper *usermapping.Service
91-
sprinklerURL string
92-
threadCache *ThreadCache // In-memory cache for fast lookups
93-
stateStore StateStore // Persistent state across restarts
94-
workspaceName string // Track workspace name for better logging
95-
eventSemaphore chan struct{} // Limits concurrent event processing (prevents overwhelming APIs)
86+
slack *slackpkg.Client
87+
github *github.Client
88+
configManager *config.Manager
89+
notifier *notify.Manager
90+
userMapper *usermapping.Service
91+
sprinklerURL string
92+
threadCache *ThreadCache // In-memory cache for fast lookups
93+
stateStore StateStore // Persistent state across restarts
94+
workspaceName string // Track workspace name for better logging
95+
eventSemaphore chan struct{} // Limits concurrent event processing (prevents overwhelming APIs)
96+
processingEvents sync.WaitGroup // Tracks in-flight event processing for graceful shutdown
9697
}
9798

9899
// StateStore interface for persistent state - allows dependency injection for testing.
99100
type StateStore interface {
100-
GetThread(owner, repo string, number int, channelID string) (ThreadInfo, bool)
101+
Thread(owner, repo string, number int, channelID string) (ThreadInfo, bool)
101102
SaveThread(owner, repo string, number int, channelID string, info ThreadInfo) error
102-
GetLastDM(userID, prURL string) (time.Time, bool)
103+
LastDM(userID, prURL string) (time.Time, bool)
103104
RecordDM(userID, prURL string, sentAt time.Time) error
104105
WasProcessed(eventKey string) bool
105106
MarkProcessed(eventKey string, ttl time.Duration) error
106-
GetLastNotification(prURL string) time.Time
107+
LastNotification(prURL string) time.Time
107108
RecordNotification(prURL string, notifiedAt time.Time) error
108109
Close() error
109110
}
@@ -1195,6 +1196,9 @@ func (c *Coordinator) handlePullRequestFromSprinkler(
11951196
logFieldOwner, owner,
11961197
logFieldRepo, repo,
11971198
"pr_number", prNumber,
1199+
"pr_state", checkResult.PullRequest.State,
1200+
"pr_draft", checkResult.PullRequest.Draft,
1201+
"pr_merged", checkResult.PullRequest.Merged,
11981202
"pr_size", checkResult.Analysis.Size,
11991203
"unresolved_comments", checkResult.Analysis.UnresolvedComments,
12001204
"checks_state", fmt.Sprintf("%+v", checkResult.Analysis.Checks),

internal/bot/bot_sprinkler.go

Lines changed: 71 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -9,6 +9,7 @@ import (
99
"strings"
1010
"time"
1111

12+
"github.com/codeGROOVE-dev/slacker/internal/state"
1213
"github.com/codeGROOVE-dev/sprinkler/pkg/client"
1314
)
1415

@@ -57,12 +58,23 @@ func (c *Coordinator) handleSprinklerEvent(ctx context.Context, event client.Eve
5758
// Try to claim this event atomically using persistent store (Datastore transaction).
5859
// This is the single source of truth for cross-instance deduplication.
5960
if err := c.stateStore.MarkProcessed(eventKey, 24*time.Hour); err != nil {
60-
slog.Info("skipping duplicate event",
61-
"organization", organization,
62-
"type", event.Type,
63-
"url", event.URL,
64-
"event_key", eventKey,
65-
"reason", "already_processed")
61+
// Check if this is a race condition vs a database error
62+
if errors.Is(err, state.ErrAlreadyProcessed) {
63+
slog.Info("skipping duplicate event - claimed by this or another instance",
64+
"organization", organization,
65+
"type", event.Type,
66+
"url", event.URL,
67+
"event_key", eventKey,
68+
"reason", "deduplication_race")
69+
} else {
70+
slog.Warn("failed to mark event as processed - database error",
71+
"organization", organization,
72+
"type", event.Type,
73+
"url", event.URL,
74+
"event_key", eventKey,
75+
"error", err,
76+
"impact", "will_skip_event")
77+
}
6678
return
6779
}
6880

@@ -76,7 +88,11 @@ func (c *Coordinator) handleSprinklerEvent(ctx context.Context, event client.Eve
7688
// Process event asynchronously after deduplication checks pass
7789
// This allows the event handler to return immediately and accept the next event
7890
// Semaphore limits concurrent processing to prevent overwhelming APIs
91+
// WaitGroup tracks in-flight events for graceful shutdown
92+
c.processingEvents.Add(1)
7993
go func() {
94+
defer c.processingEvents.Done()
95+
8096
// Acquire semaphore slot (blocks if 10 events already processing)
8197
c.eventSemaphore <- struct{}{}
8298
defer func() { <-c.eventSemaphore }() // Release slot when done
@@ -166,6 +182,47 @@ func (c *Coordinator) handleSprinklerEvent(ctx context.Context, event client.Eve
166182
}() // Close the goroutine
167183
}
168184

185+
// waitForEventProcessing waits for all in-flight events to complete during shutdown.
186+
// Returns immediately if no events are being processed.
187+
func (c *Coordinator) waitForEventProcessing(organization string, maxWait time.Duration) {
188+
// Check if any events are being processed
189+
queueLen := len(c.eventSemaphore)
190+
if queueLen == 0 {
191+
slog.Info("no events in processing queue, shutdown can proceed immediately",
192+
"organization", organization)
193+
return
194+
}
195+
196+
slog.Warn("waiting for in-flight events to complete before shutdown",
197+
"organization", organization,
198+
"events_in_queue", queueLen,
199+
"max_wait_seconds", maxWait.Seconds())
200+
201+
// Create a channel to signal when all events are done
202+
done := make(chan struct{})
203+
go func() {
204+
c.processingEvents.Wait()
205+
close(done)
206+
}()
207+
208+
// Wait for events to complete or timeout
209+
select {
210+
case <-done:
211+
slog.Info("all in-flight events completed successfully",
212+
"organization", organization,
213+
"graceful_shutdown", true)
214+
case <-time.After(maxWait):
215+
remaining := len(c.eventSemaphore)
216+
slog.Warn("shutdown timeout reached, proceeding with remaining events in queue",
217+
"organization", organization,
218+
"events_still_processing", remaining,
219+
"waited_seconds", maxWait.Seconds(),
220+
"impact", "these events may be incomplete",
221+
"recovery", "polling will catch them in next 5min cycle",
222+
"graceful_shutdown", false)
223+
}
224+
}
225+
169226
// handleAuthError handles authentication errors by refreshing the token and recreating the client.
170227
func (c *Coordinator) handleAuthError(
171228
ctx context.Context,
@@ -302,6 +359,8 @@ func (c *Coordinator) RunWithSprinklerClient(ctx context.Context) error {
302359
slog.Info("sprinkler client context cancelled, stopping gracefully",
303360
"organization", organization,
304361
"total_attempts", attempts)
362+
// Wait for in-flight events (up to 8 seconds, leaving 2s for HTTP shutdown)
363+
c.waitForEventProcessing(organization, 8*time.Second)
305364
return nil
306365
}
307366

@@ -311,6 +370,8 @@ func (c *Coordinator) RunWithSprinklerClient(ctx context.Context) error {
311370
slog.Info("context cancelled, stopping sprinkler client",
312371
"organization", organization,
313372
"context_error", ctxErr)
373+
// Wait for in-flight events (up to 8 seconds)
374+
c.waitForEventProcessing(organization, 8*time.Second)
314375
return ctxErr
315376
}
316377

@@ -347,6 +408,7 @@ func (c *Coordinator) RunWithSprinklerClient(ctx context.Context) error {
347408
"will_retry", true)
348409
select {
349410
case <-ctx.Done():
411+
c.waitForEventProcessing(organization, 8*time.Second)
350412
return ctx.Err()
351413
case <-time.After(retryDelay):
352414
continue
@@ -371,6 +433,7 @@ func (c *Coordinator) RunWithSprinklerClient(ctx context.Context) error {
371433
select {
372434
case <-ctx.Done():
373435
slog.Info("context cancelled during retry wait", "organization", organization)
436+
c.waitForEventProcessing(organization, 8*time.Second)
374437
return ctx.Err()
375438
case <-time.After(retryDelay):
376439
// Exponential backoff capped at maxRetryDelay
@@ -391,6 +454,7 @@ func (c *Coordinator) RunWithSprinklerClient(ctx context.Context) error {
391454
if ctxErr := ctx.Err(); ctxErr != nil {
392455
slog.Info("sprinkler client stopped cleanly due to context cancellation",
393456
"organization", organization)
457+
c.waitForEventProcessing(organization, 8*time.Second)
394458
return ctxErr
395459
}
396460

@@ -407,6 +471,7 @@ func (c *Coordinator) RunWithSprinklerClient(ctx context.Context) error {
407471
// This might be network hiccup or server restart
408472
select {
409473
case <-ctx.Done():
474+
c.waitForEventProcessing(organization, 8*time.Second)
410475
return ctx.Err()
411476
case <-time.After(5 * time.Second):
412477
slog.Info("restarting after unexpected clean disconnect",

internal/bot/polling.go

Lines changed: 12 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -198,6 +198,9 @@ func (c *Coordinator) reconcilePR(ctx context.Context, pr *github.PRSnapshot) er
198198

199199
slog.Debug("turnclient analysis complete",
200200
"pr", fmt.Sprintf("%s/%s#%d", pr.Owner, pr.Repo, pr.Number),
201+
"pr_state", checkResult.PullRequest.State,
202+
"pr_draft", checkResult.PullRequest.Draft,
203+
"pr_merged", checkResult.PullRequest.Merged,
201204
"ready_to_merge", checkResult.Analysis.ReadyToMerge,
202205
"approved", checkResult.Analysis.Approved,
203206
"next_action_count", len(checkResult.Analysis.NextAction))
@@ -271,12 +274,15 @@ func (c *Coordinator) updateClosedPRThread(ctx context.Context, pr *github.PRSna
271274
continue
272275
}
273276

274-
info, ok := c.stateStore.GetThread(pr.Owner, pr.Repo, pr.Number, id)
277+
info, ok := c.stateStore.Thread(pr.Owner, pr.Repo, pr.Number, id)
275278
if !ok {
276279
slog.Debug("no thread found for closed PR in channel",
277280
"pr", prKey,
278281
"channel", ch,
279-
"channel_id", id)
282+
"channel_id", id,
283+
"pr_state", pr.State,
284+
"pr_updated_at", pr.UpdatedAt,
285+
"possible_reason", "PR closed before thread created or thread in different channel")
280286
continue
281287
}
282288

@@ -351,7 +357,9 @@ func (c *Coordinator) StartupReconciliation(ctx context.Context) {
351357
slog.Info("🔄 STARTUP RECONCILIATION STARTED",
352358
"org", org,
353359
"purpose", "catch up on missed notifications during downtime",
354-
"window", "24h")
360+
"window", "24h",
361+
"scope", "open_prs_only",
362+
"note", "closed PRs handled by polling cycle")
355363

356364
// Get current GitHub token
357365
token := c.github.InstallationToken(ctx)
@@ -400,7 +408,7 @@ func (c *Coordinator) StartupReconciliation(ctx context.Context) {
400408
}
401409

402410
// Check notification state
403-
lastNotified := c.stateStore.GetLastNotification(pr.URL)
411+
lastNotified := c.stateStore.LastNotification(pr.URL)
404412

405413
// Determine if we should notify
406414
var reason string

internal/config/config.go

Lines changed: 6 additions & 26 deletions
Original file line numberDiff line numberDiff line change
@@ -307,24 +307,7 @@ func (m *Manager) LoadConfig(ctx context.Context, org string) error {
307307

308308
var config RepoConfig
309309
if err := yaml.Unmarshal([]byte(configContent), &config); err != nil {
310-
defaultConfig := &RepoConfig{
311-
Channels: make(map[string]struct {
312-
ReminderDMDelay *int `yaml:"reminder_dm_delay"` // Optional: override global delay for this channel (0 = disabled)
313-
Repos []string `yaml:"repos"`
314-
Mute bool `yaml:"mute"`
315-
}),
316-
Global: struct {
317-
TeamID string `yaml:"team_id"`
318-
EmailDomain string `yaml:"email_domain"`
319-
ReminderDMDelay int `yaml:"reminder_dm_delay"`
320-
DailyReminders bool `yaml:"daily_reminders"`
321-
}{
322-
TeamID: "",
323-
EmailDomain: "",
324-
ReminderDMDelay: defaultReminderDMDelayMinutes,
325-
DailyReminders: true,
326-
},
327-
}
310+
defaultConfig := createDefaultConfig()
328311
m.configs[org] = defaultConfig
329312
m.cache.set(org, defaultConfig)
330313

@@ -356,26 +339,23 @@ func (m *Manager) LoadConfig(ctx context.Context, org string) error {
356339
mutedChannels++
357340
}
358341
totalRepos += len(channelConfig.Repos)
342+
343+
hasWildcard := false
359344
for _, repo := range channelConfig.Repos {
360345
if repo == "*" {
361346
wildcardChannels++
347+
hasWildcard = true
362348
break
363349
}
364350
}
351+
365352
slog.Debug("channel configuration loaded",
366353
logFieldOrg, org,
367354
"channel", channelName,
368355
"repos_count", len(channelConfig.Repos),
369356
"repos", channelConfig.Repos,
370357
"muted", channelConfig.Mute,
371-
"has_wildcard", func() bool {
372-
for _, r := range channelConfig.Repos {
373-
if r == "*" {
374-
return true
375-
}
376-
}
377-
return false
378-
}())
358+
"has_wildcard", hasWildcard)
379359
}
380360

381361
m.configs[org] = &config

0 commit comments

Comments
 (0)