Skip to content

Commit 212a9d4

Browse files
authored
Merge pull request #74 from codeGROOVE-dev/reliable
Add /r2r report
2 parents 0f3b02e + 0678435 commit 212a9d4

25 files changed

+1523
-1943
lines changed

cmd/server/main.go

Lines changed: 6 additions & 21 deletions
Original file line numberDiff line numberDiff line change
@@ -265,6 +265,10 @@ func run(ctx context.Context, cancel context.CancelFunc, cfg *config.ServerConfi
265265
homeHandler := slack.NewHomeHandler(slackManager, githubManager, configManager, stateStore, reverseMapping)
266266
slackManager.SetHomeViewHandler(homeHandler.HandleAppHomeOpened)
267267

268+
// Initialize report handler for /r2r report slash command
269+
reportHandler := slack.NewReportHandler(slackManager, githubManager, stateStore, reverseMapping)
270+
slackManager.SetReportHandler(reportHandler.HandleReportCommand)
271+
268272
// Initialize OAuth handler for Slack app installation.
269273
// These credentials are needed for the OAuth flow.
270274
slackClientID := os.Getenv("SLACK_CLIENT_ID")
@@ -692,10 +696,6 @@ func runBotCoordinators(
692696
lastHealthCheck: time.Now(),
693697
}
694698

695-
// Initialize daily digest scheduler
696-
//nolint:revive // line length acceptable for initialization
697-
dailyDigest := notify.NewDailyDigestScheduler(notifier, github.WrapManager(githubManager), configManager, stateStore, notify.WrapSlackManager(slackManager))
698-
699699
// Start initial coordinators
700700
cm.startCoordinators(ctx)
701701

@@ -712,19 +712,10 @@ func runBotCoordinators(
712712
defer healthCheckTicker.Stop()
713713

714714
// Poll for PRs every 5 minutes (safety net for missed sprinkler events)
715+
// Daily reports are checked during each poll cycle (6am-11:30am window)
715716
pollTicker := time.NewTicker(5 * time.Minute)
716717
defer pollTicker.Stop()
717718

718-
// Check for daily digest candidates every hour
719-
dailyDigestTicker := time.NewTicker(1 * time.Hour)
720-
defer dailyDigestTicker.Stop()
721-
722-
// Run daily digest check immediately on startup
723-
// (in case server starts during someone's 8-9am window)
724-
go func() {
725-
dailyDigest.CheckAndSend(ctx)
726-
}()
727-
728719
// Setup state cleanup ticker (hourly)
729720
cleanupTicker := time.NewTicker(1 * time.Hour)
730721
defer cleanupTicker.Stop()
@@ -754,15 +745,9 @@ func runBotCoordinators(
754745
cm.handleRefreshInstallations(ctx)
755746

756747
case <-pollTicker.C:
757-
// Poll all active coordinators
748+
// Poll all active coordinators (includes daily report checks)
758749
cm.handlePolling(ctx)
759750

760-
case <-dailyDigestTicker.C:
761-
// Check for daily digest candidates across all orgs
762-
go func() {
763-
dailyDigest.CheckAndSend(ctx)
764-
}()
765-
766751
case <-cleanupTicker.C:
767752
// Periodic cleanup of old state data
768753
//nolint:contextcheck // Background cleanup should complete even during shutdown

pkg/bot/bot.go

Lines changed: 6 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -76,7 +76,7 @@ type Coordinator struct {
7676

7777
// StateStore interface for persistent state - allows dependency injection for testing.
7878
//
79-
//nolint:interfacebloat // 15 methods needed for complete state management
79+
//nolint:interfacebloat // 20 methods needed for complete state management
8080
type StateStore interface {
8181
Thread(ctx context.Context, owner, repo string, number int, channelID string) (cache.ThreadInfo, bool)
8282
SaveThread(ctx context.Context, owner, repo string, number int, channelID string, info cache.ThreadInfo) error
@@ -85,13 +85,18 @@ type StateStore interface {
8585
DMMessage(ctx context.Context, userID, prURL string) (state.DMInfo, bool)
8686
SaveDMMessage(ctx context.Context, userID, prURL string, info state.DMInfo) error
8787
ListDMUsers(ctx context.Context, prURL string) []string
88+
LastDigest(ctx context.Context, userID, date string) (time.Time, bool)
89+
RecordDigest(ctx context.Context, userID, date string, sentAt time.Time) error
8890
QueuePendingDM(ctx context.Context, dm *state.PendingDM) error
8991
PendingDMs(ctx context.Context, before time.Time) ([]state.PendingDM, error)
9092
RemovePendingDM(ctx context.Context, id string) error
9193
WasProcessed(ctx context.Context, eventKey string) bool
9294
MarkProcessed(ctx context.Context, eventKey string, ttl time.Duration) error
9395
LastNotification(ctx context.Context, prURL string) time.Time
9496
RecordNotification(ctx context.Context, prURL string, notifiedAt time.Time) error
97+
LastReportSent(ctx context.Context, userID string) (time.Time, bool)
98+
RecordReportSent(ctx context.Context, userID string, sentAt time.Time) error
99+
Cleanup(ctx context.Context) error
95100
Close() error
96101
}
97102

pkg/bot/coordinator_test_helpers.go

Lines changed: 55 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -208,6 +208,39 @@ func (m *mockStateStore) RemovePendingDM(ctx context.Context, id string) error {
208208
return nil
209209
}
210210

211+
func (m *mockStateStore) LastDigest(_ context.Context, _ /* userID */, _ /* date */ string) (time.Time, bool) {
212+
m.mu.Lock()
213+
defer m.mu.Unlock()
214+
// Simple mock - always return false
215+
return time.Time{}, false
216+
}
217+
218+
func (m *mockStateStore) RecordDigest(_ context.Context, _ /* userID */, _ /* date */ string, _ /* sentAt */ time.Time) error {
219+
m.mu.Lock()
220+
defer m.mu.Unlock()
221+
// Simple mock - no-op
222+
return nil
223+
}
224+
225+
func (m *mockStateStore) LastReportSent(_ context.Context, _ /* userID */ string) (time.Time, bool) {
226+
m.mu.Lock()
227+
defer m.mu.Unlock()
228+
// Simple mock - always return false (no reports sent)
229+
return time.Time{}, false
230+
}
231+
232+
func (m *mockStateStore) RecordReportSent(_ context.Context, _ /* userID */ string, _ /* sentAt */ time.Time) error {
233+
m.mu.Lock()
234+
defer m.mu.Unlock()
235+
// Simple mock - no-op
236+
return nil
237+
}
238+
239+
func (*mockStateStore) Cleanup(_ context.Context) error {
240+
// Simple mock - no-op
241+
return nil
242+
}
243+
211244
func (*mockStateStore) Close() error {
212245
return nil
213246
}
@@ -377,6 +410,28 @@ func (m *mockSlackClient) SendDirectMessage(ctx context.Context, userID, text st
377410
return "D" + userID, "1234567890.123456", nil
378411
}
379412

413+
// SendDirectMessageWithBlocks sends a Block Kit DM to a user.
414+
func (*mockSlackClient) SendDirectMessageWithBlocks(
415+
_ context.Context,
416+
userID string,
417+
_ []slack.Block,
418+
) (dmChannelID, messageTS string, err error) {
419+
// Simple mock - just return success
420+
return "D" + userID, "1234567890.123456", nil
421+
}
422+
423+
// IsUserActive checks if a user is currently active.
424+
func (*mockSlackClient) IsUserActive(_ context.Context, _ /* userID */ string) bool {
425+
// Simple mock - always return true (active)
426+
return true
427+
}
428+
429+
// UserTimezone returns the user's IANA timezone.
430+
func (*mockSlackClient) UserTimezone(_ context.Context, _ /* userID */ string) (string, error) {
431+
// Simple mock - return America/New_York
432+
return "America/New_York", nil
433+
}
434+
380435
// IsUserInChannel checks if a user is in a channel.
381436
func (m *mockSlackClient) IsUserInChannel(ctx context.Context, channelID, userID string) bool {
382437
if m.isUserInChannelFunc != nil {

pkg/bot/interfaces.go

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -16,13 +16,16 @@ import (
1616
// SlackClient defines Slack operations needed by bot.
1717
// Small interface - only methods we actually call.
1818
//
19-
//nolint:interfacebloat // 13 methods needed for Slack integration
19+
//nolint:interfacebloat // 16 methods needed for Slack integration
2020
type SlackClient interface {
2121
PostThread(ctx context.Context, channelID, text string, attachments []slack.Attachment) (string, error)
2222
UpdateMessage(ctx context.Context, channelID, timestamp, text string) error
2323
UpdateDMMessage(ctx context.Context, userID, prURL, text string) error
2424
SendDirectMessage(ctx context.Context, userID, text string) (dmChannelID, messageTS string, err error)
25+
SendDirectMessageWithBlocks(ctx context.Context, userID string, blocks []slack.Block) (dmChannelID, messageTS string, err error)
2526
IsUserInChannel(ctx context.Context, channelID, userID string) bool
27+
IsUserActive(ctx context.Context, userID string) bool
28+
UserTimezone(ctx context.Context, userID string) (string, error)
2629
FindDMMessagesInHistory(ctx context.Context, userID, prURL string, since time.Time) ([]slackapi.DMLocation, error)
2730
ChannelHistory(ctx context.Context, channelID string, oldest, latest string, limit int) (*slack.GetConversationHistoryResponse, error)
2831
ResolveChannelID(ctx context.Context, channelName string) string

pkg/bot/polling.go

Lines changed: 158 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -8,8 +8,11 @@ import (
88
"os"
99
"time"
1010

11+
"github.com/codeGROOVE-dev/slacker/pkg/dailyreport"
1112
"github.com/codeGROOVE-dev/slacker/pkg/github"
13+
"github.com/codeGROOVE-dev/slacker/pkg/home"
1214
"github.com/codeGROOVE-dev/turnclient/pkg/turn"
15+
gogithub "github.com/google/go-github/v50/github"
1316
)
1417

1518
// makePollEventKey creates an event key for poll-based PR processing.
@@ -192,6 +195,9 @@ func (c *Coordinator) pollAndReconcileWithSearcher(ctx context.Context, searcher
192195
"processed", successCount,
193196
"errors", errorCount,
194197
"next_poll", "5m")
198+
199+
// Check daily reports for users involved in these PRs
200+
c.checkDailyReports(ctx, org, prs)
195201
}
196202

197203
// reconcilePR checks a single PR and sends notifications if needed.
@@ -439,3 +445,155 @@ func (c *Coordinator) StartupReconciliation(ctx context.Context) {
439445
"skipped", skippedCount,
440446
"errors", errorCount)
441447
}
448+
449+
// extractUniqueGitHubUsers extracts all unique GitHub usernames involved in PRs.
450+
// Currently extracts PR authors. The FetchDashboard call will determine which users
451+
// have incoming PRs to review (as reviewers/assignees).
452+
func extractUniqueGitHubUsers(prs []github.PRSnapshot) map[string]bool {
453+
users := make(map[string]bool)
454+
455+
for i := range prs {
456+
pr := &prs[i]
457+
458+
// Add author
459+
if pr.Author != "" {
460+
users[pr.Author] = true
461+
}
462+
}
463+
464+
return users
465+
}
466+
467+
// checkDailyReports checks if users involved in PRs should receive daily reports.
468+
// Efficiently extracts unique GitHub users from polled PRs instead of iterating all workspace users.
469+
// Reports are sent between 6am-11:30am user local time, with 23+ hour intervals.
470+
func (c *Coordinator) checkDailyReports(ctx context.Context, org string, prs []github.PRSnapshot) {
471+
// Check if daily reports are enabled for this org
472+
cfg, exists := c.configManager.Config(org)
473+
if !exists {
474+
slog.Debug("skipping daily reports - no config found", "org", org)
475+
return
476+
}
477+
478+
if cfg.Global.DisableDailyReport {
479+
slog.Debug("daily reports disabled for org", "org", org)
480+
return
481+
}
482+
483+
// Get domain for user mapping
484+
domain := cfg.Global.EmailDomain
485+
486+
// Extract unique GitHub users from PRs (authors, reviewers, assignees)
487+
githubUsers := extractUniqueGitHubUsers(prs)
488+
if len(githubUsers) == 0 {
489+
slog.Debug("no users involved in PRs, skipping daily reports", "org", org)
490+
return
491+
}
492+
493+
slog.Debug("checking daily reports for PR-involved users",
494+
"org", org,
495+
"unique_github_users", len(githubUsers),
496+
"window", "6am-11:30am local time",
497+
"min_interval", "23 hours")
498+
499+
// Get GitHub client for dashboard fetching
500+
token := c.github.InstallationToken(ctx)
501+
if token == "" {
502+
slog.Warn("skipping daily reports - no GitHub token", "org", org)
503+
return
504+
}
505+
506+
ghClient, ok := c.github.Client().(*gogithub.Client)
507+
if !ok {
508+
slog.Error("skipping daily reports - failed to get GitHub client")
509+
return
510+
}
511+
512+
// Create daily report sender and dashboard fetcher
513+
sender := dailyreport.NewSender(c.stateStore, c.slack)
514+
fetcher := home.NewFetcher(ghClient, c.stateStore, token, "ready-to-review[bot]")
515+
516+
sentCount := 0
517+
skippedCount := 0
518+
errorCount := 0
519+
520+
for githubUsername := range githubUsers {
521+
// Map GitHub user to Slack user ID
522+
slackUserID, err := c.userMapper.SlackHandle(ctx, githubUsername, org, domain)
523+
if err != nil || slackUserID == "" {
524+
slog.Debug("skipping daily report - no Slack mapping",
525+
"github_user", githubUsername,
526+
"error", err)
527+
skippedCount++
528+
continue
529+
}
530+
531+
// Fetch user's dashboard (incoming/outgoing PRs)
532+
dashboard, err := fetcher.FetchDashboard(ctx, githubUsername, []string{org})
533+
if err != nil {
534+
slog.Debug("skipping daily report - dashboard fetch failed",
535+
"github_user", githubUsername,
536+
"slack_user", slackUserID,
537+
"error", err)
538+
errorCount++
539+
continue
540+
}
541+
542+
// Build user blocking info
543+
userInfo := dailyreport.UserBlockingInfo{
544+
GitHubUsername: githubUsername,
545+
SlackUserID: slackUserID,
546+
IncomingPRs: dashboard.IncomingPRs,
547+
OutgoingPRs: dashboard.OutgoingPRs,
548+
}
549+
550+
// Check if should send report
551+
if !sender.ShouldSendReport(ctx, userInfo) {
552+
// Record timestamp even if no report sent (no PRs or outside window)
553+
// This prevents checking this user again for 23 hours
554+
if len(dashboard.IncomingPRs) == 0 && len(dashboard.OutgoingPRs) == 0 {
555+
if err := c.stateStore.RecordReportSent(ctx, slackUserID, time.Now()); err != nil {
556+
slog.Debug("failed to record empty report timestamp",
557+
"github_user", githubUsername,
558+
"slack_user", slackUserID,
559+
"error", err)
560+
}
561+
}
562+
skippedCount++
563+
continue
564+
}
565+
566+
// Send the report
567+
if err := sender.SendReport(ctx, userInfo); err != nil {
568+
slog.Warn("failed to send daily report",
569+
"github_user", githubUsername,
570+
"slack_user", slackUserID,
571+
"error", err)
572+
errorCount++
573+
continue
574+
}
575+
576+
slog.Info("sent daily report",
577+
"github_user", githubUsername,
578+
"slack_user", slackUserID,
579+
"incoming_prs", len(dashboard.IncomingPRs),
580+
"outgoing_prs", len(dashboard.OutgoingPRs))
581+
sentCount++
582+
583+
// Rate limit to avoid overwhelming Slack/GitHub APIs
584+
select {
585+
case <-ctx.Done():
586+
slog.Info("daily report check canceled", "org", org)
587+
return
588+
case <-time.After(500 * time.Millisecond):
589+
}
590+
}
591+
592+
if sentCount > 0 || errorCount > 0 {
593+
slog.Info("daily report check complete",
594+
"org", org,
595+
"sent", sentCount,
596+
"skipped", skippedCount,
597+
"errors", errorCount)
598+
}
599+
}

0 commit comments

Comments
 (0)