Merge pull request #66 from codeGROOVE-dev/reliable

Add support for DMs

Author: tstromberg

.github/workflows/test.yml

@@ -18,7 +18,7 @@ jobs:
       - name: Set up Go
         uses: actions/setup-go@v5
         with:
-          go-version: '1.23'
+          go-version: 'stable'
           cache: true
 
       - name: Install dependencies

Makefile

@@ -16,35 +16,7 @@ build-registrar:
 
 # Run tests with race detection and coverage
 test:
-	@echo "Running tests with race detection and coverage..."
-	@go test -v -race -coverprofile=coverage.out -covermode=atomic ./...
-	@echo ""
-	@echo "Coverage by package:"
-	@go test -coverprofile=coverage.out -covermode=atomic ./... 2>&1 | grep -E "coverage:" | awk '{print $$2 "\t" $$5}' | column -t
-	@echo ""
-	@echo "Checking for packages below 80% coverage..."
-	@failed=0; \
-	packages=$$(go list ./... | grep -v "/cmd/"); \
-	for pkg in $$packages; do \
-		output=$$(go test -coverprofile=/dev/null "$$pkg" 2>&1); \
-		if echo "$$output" | grep -q "\[no test files\]"; then \
-			continue; \
-		fi; \
-		coverage=$$(echo "$$output" | grep "coverage:" | awk '{print $$5}' | sed 's/%//'); \
-		if [ -n "$$coverage" ] && [ "$$coverage" != "statements" ]; then \
-			pkg_short=$$(echo "$$pkg" | sed 's|github.com/codeGROOVE-dev/slacker/||'); \
-			if [ "$$(echo "$$coverage < 80.0" | bc -l 2>/dev/null || echo 0)" -eq 1 ]; then \
-				echo "❌ FAIL: $$pkg_short has $$coverage% coverage (minimum: 80%)"; \
-				failed=1; \
-			fi; \
-		fi; \
-	done; \
-	if [ $$failed -eq 1 ]; then \
-		echo ""; \
-		echo "Coverage check failed. All packages must have at least 80% coverage."; \
-		exit 1; \
-	fi
-	@echo "✅ All packages meet 80% coverage threshold"
+	go test -v -race -cover ./...
 
 # Format code
 fmt:

cmd/server/main.go

@@ -162,24 +162,7 @@ func run(ctx context.Context, cancel context.CancelFunc, cfg *config.ServerConfi
 	slackManager := slack.NewManager(cfg.SlackSigningSecret)
 
 	// Initialize state store (in-memory + Datastore or JSON for persistence).
-	//nolint:interfacebloat // Interface mirrors state.Store for local type safety
-	var stateStore interface {
-		Thread(owner, repo string, number int, channelID string) (state.ThreadInfo, bool)
-		SaveThread(owner, repo string, number int, channelID string, info state.ThreadInfo) error
-		LastDM(userID, prURL string) (time.Time, bool)
-		RecordDM(userID, prURL string, sentAt time.Time) error
-		DMMessage(userID, prURL string) (state.DMInfo, bool)
-		SaveDMMessage(userID, prURL string, info state.DMInfo) error
-		ListDMUsers(prURL string) []string
-		LastDigest(userID, date string) (time.Time, bool)
-		RecordDigest(userID, date string, sentAt time.Time) error
-		WasProcessed(eventKey string) bool
-		MarkProcessed(eventKey string, ttl time.Duration) error
-		LastNotification(prURL string) time.Time
-		RecordNotification(prURL string, notifiedAt time.Time) error
-		Cleanup() error
-		Close() error
-	}
+	var stateStore state.Store
 
 	// Check if Datastore should be used via DATASTORE=<database-id>
 	// Examples:
@@ -256,7 +239,7 @@ func run(ctx context.Context, cancel context.CancelFunc, cfg *config.ServerConfi
 	slog.Info("configured Slack manager with state store for DM tracking")
 
 	// Initialize notification manager for multi-workspace notifications.
-	notifier := notify.New(notify.WrapSlackManager(slackManager), configManager)
+	notifier := notify.New(notify.WrapSlackManager(slackManager), configManager, stateStore)
 
 	// Initialize event router for multi-workspace event handling.
 	eventRouter := slack.NewEventRouter(slackManager)
@@ -679,23 +662,7 @@ func runBotCoordinators(
 	githubManager *github.Manager,
 	configManager *config.Manager,
 	notifier *notify.Manager,
-	stateStore interface {
-		Thread(owner, repo string, number int, channelID string) (state.ThreadInfo, bool)
-		SaveThread(owner, repo string, number int, channelID string, info state.ThreadInfo) error
-		LastDM(userID, prURL string) (time.Time, bool)
-		RecordDM(userID, prURL string, sentAt time.Time) error
-		DMMessage(userID, prURL string) (state.DMInfo, bool)
-		SaveDMMessage(userID, prURL string, info state.DMInfo) error
-		ListDMUsers(prURL string) []string
-		LastDigest(userID, date string) (time.Time, bool)
-		RecordDigest(userID, date string, sentAt time.Time) error
-		WasProcessed(eventKey string) bool
-		MarkProcessed(eventKey string, ttl time.Duration) error
-		LastNotification(prURL string) time.Time
-		RecordNotification(prURL string, notifiedAt time.Time) error
-		Cleanup() error
-		Close() error
-	},
+	stateStore state.Store,
 	sprinklerURL string,
 ) error {
 	cm := &coordinatorManager{

go.mod

@@ -26,6 +26,7 @@ require (
 	github.com/cloudflare/circl v1.6.1 // indirect
 	github.com/google/go-cmp v0.7.0 // indirect
 	github.com/google/go-querystring v1.1.0 // indirect
+	github.com/google/uuid v1.6.0 // indirect
 	github.com/gorilla/websocket v1.5.3 // indirect
 	github.com/kr/pretty v0.3.1 // indirect
 	github.com/rogpeppe/go-internal v1.14.1 // indirect

go.sum

@@ -30,6 +30,8 @@ github.com/google/go-github/v50 v50.2.0 h1:j2FyongEHlO9nxXLc+LP3wuBSVU9mVxfpdYUe
 github.com/google/go-github/v50 v50.2.0/go.mod h1:VBY8FB6yPIjrtKhozXv4FQupxKLS6H4m6xFZlT43q8Q=
 github.com/google/go-querystring v1.1.0 h1:AnCroh3fv4ZBgVIf1Iwtovgjaw/GiKJo8M8yD/fhyJ8=
 github.com/google/go-querystring v1.1.0/go.mod h1:Kcdr2DB4koayq7X8pmAG4sNG59So17icRSOU623lUBU=
+github.com/google/uuid v1.6.0 h1:NIvaJDMOsjHA8n1jAhLSgzrAzy1Hgr+hNrb57e+94F0=
+github.com/google/uuid v1.6.0/go.mod h1:TIyPZe4MgqvfeYDBFedMoGGpEw/LqOeaOT+nhxU+yHo=
 github.com/gorilla/mux v1.8.1 h1:TuBL49tXwgrFYWhqrNgrUNEY92u81SPhu7sTdzQEiWY=
 github.com/gorilla/mux v1.8.1/go.mod h1:AKf9I4AEqPTmMytcMc0KkNouC66V3BtZ4qD5fmWSiMQ=
 github.com/gorilla/websocket v1.5.3 h1:saDtZ6Pbx/0u+bgYQ3q96pZgCzfhKXGPqt7kZ72aNNg=

pkg/bot/integration_test.go

@@ -10,6 +10,7 @@ import (
 	"github.com/codeGROOVE-dev/slacker/pkg/notify"
 	"github.com/codeGROOVE-dev/slacker/pkg/slack"
 	"github.com/codeGROOVE-dev/slacker/pkg/slacktest"
+	"github.com/codeGROOVE-dev/slacker/pkg/state"
 	"github.com/codeGROOVE-dev/slacker/pkg/usermapping"
 	"github.com/codeGROOVE-dev/turnclient/pkg/turn"
 	slackapi "github.com/slack-go/slack"
@@ -231,7 +232,10 @@ func TestDMDelayLogicIntegration(t *testing.T) {
 		dmDelay: 65, // 65 minute delay
 	}
 
-	notifier := notify.New(notify.WrapSlackManager(slackManager), configMgr)
+	// Create in-memory store for pending DMs
+	store := state.NewMemoryStore()
+
+	notifier := notify.New(notify.WrapSlackManager(slackManager), configMgr, store)
 
 	prInfo := notify.PRInfo{
 		Owner:         "test",
@@ -291,7 +295,8 @@ func TestDMDelayLogicIntegration(t *testing.T) {
 			// Create fresh tracker with initialized maps
 			notifier.Tracker = &notify.NotificationTracker{}
 			// Initialize the tracker by creating a new notifier
-			notifier = notify.New(notify.WrapSlackManager(slackManager), configMgr)
+			store = state.NewMemoryStore() // Reset store as well
+			notifier = notify.New(notify.WrapSlackManager(slackManager), configMgr, store)
 
 			// Setup test scenario
 			if tt.setupFunc != nil {

pkg/config/config.go

@@ -374,6 +374,7 @@ func (m *Manager) LoadConfig(ctx context.Context, org string) error {
 			"team_id":             config.Global.TeamID,
 			"email_domain":        config.Global.EmailDomain,
 			"daily_reminders":     config.Global.DailyReminders,
+			"reminder_dm_delay":   config.Global.ReminderDMDelay,
 			"total_channels":      len(config.Channels),
 			"muted_channels":      muted,
 			"wildcard_channels":   wildcard,
@@ -575,20 +576,36 @@ func (m *Manager) ReminderDMDelay(org, channel string) int {
 
 	config, exists := m.configs[org]
 	if !exists {
+		slog.Debug("no config for org - using default delay",
+			logFieldOrg, org,
+			"default_delay_mins", defaultReminderDMDelayMinutes)
 		return defaultReminderDMDelayMinutes
 	}
 
 	// Check for channel-specific override
 	if channelConfig, ok := config.Channels[channel]; ok {
 		if channelConfig.ReminderDMDelay != nil {
+			slog.Debug("using channel-specific reminder delay",
+				logFieldOrg, org,
+				"channel", channel,
+				"delay_mins", *channelConfig.ReminderDMDelay)
 			return *channelConfig.ReminderDMDelay
 		}
 	}
 
 	// Return global setting (or default if not set)
 	if config.Global.ReminderDMDelay > 0 {
+		slog.Debug("using global reminder delay",
+			logFieldOrg, org,
+			"channel", channel,
+			"delay_mins", config.Global.ReminderDMDelay)
 		return config.Global.ReminderDMDelay
 	}
+	slog.Debug("global delay is 0 or unset - using default",
+		logFieldOrg, org,
+		"channel", channel,
+		"global_value", config.Global.ReminderDMDelay,
+		"default_delay_mins", defaultReminderDMDelayMinutes)
 	return defaultReminderDMDelayMinutes
 }
 

pkg/config/config_test.go

@@ -1071,6 +1071,112 @@ channels: [1, 2, 3]
 	}
 }
 
+func TestManager_LoadConfigCodeGROOVEProdConfig(t *testing.T) {
+	// Test with actual production config from codeGROOVE-dev/.codeGROOVE/slack.yaml
+	prodYAML := `global:
+    team_id: T09CJ7X7T7Y
+    email_domain: codegroove.dev
+    reminder_dm_delay: 1
+
+channels:
+    goose:
+      mute: true
+
+    all-codegroove:
+      repos:
+        - "*"
+
+    social:
+      repos:
+        - goose
+        - sprinkler
+        - slacker
+`
+
+	handler := func(w http.ResponseWriter, r *http.Request) {
+		content := base64.StdEncoding.EncodeToString([]byte(prodYAML))
+		encoding := "base64"
+		response := github.RepositoryContent{
+			Type:     github.String("file"),
+			Content:  &content,
+			Encoding: &encoding,
+		}
+		w.Header().Set("Content-Type", "application/json")
+		if err := json.NewEncoder(w).Encode(response); err != nil {
+			t.Errorf("failed to encode response: %v", err)
+		}
+	}
+
+	client, server := createTestGitHubClient(handler)
+	defer server.Close()
+
+	m := New()
+	m.SetGitHubClient("codeGROOVE-dev", client)
+
+	ctx := context.Background()
+	err := m.LoadConfig(ctx, "codeGROOVE-dev")
+	if err != nil {
+		t.Fatalf("unexpected error loading production config: %v", err)
+	}
+
+	// Verify config was loaded correctly
+	cfg, exists := m.Config("codeGROOVE-dev")
+	if !exists {
+		t.Fatal("expected config to exist after loading")
+	}
+	if cfg.Global.TeamID != "T09CJ7X7T7Y" {
+		t.Errorf("expected TeamID T09CJ7X7T7Y, got %q", cfg.Global.TeamID)
+	}
+	if cfg.Global.EmailDomain != "codegroove.dev" {
+		t.Errorf("expected email domain codegroove.dev, got %q", cfg.Global.EmailDomain)
+	}
+	if cfg.Global.ReminderDMDelay != 1 {
+		t.Errorf("expected reminder delay 1 minute, got %d", cfg.Global.ReminderDMDelay)
+	}
+	if len(cfg.Channels) != 3 {
+		t.Errorf("expected 3 channels, got %d", len(cfg.Channels))
+	}
+
+	// Verify goose channel is muted
+	gooseChannel, exists := cfg.Channels["goose"]
+	if !exists {
+		t.Error("expected goose channel to exist")
+	}
+	if !gooseChannel.Mute {
+		t.Error("expected goose channel to be muted")
+	}
+
+	// Verify all-codegroove has wildcard
+	allChannel, exists := cfg.Channels["all-codegroove"]
+	if !exists {
+		t.Error("expected all-codegroove channel to exist")
+	}
+	if len(allChannel.Repos) != 1 || allChannel.Repos[0] != "*" {
+		t.Errorf("expected all-codegroove to have wildcard repo, got %v", allChannel.Repos)
+	}
+
+	// Verify social channel repos
+	socialChannel, exists := cfg.Channels["social"]
+	if !exists {
+		t.Error("expected social channel to exist")
+	}
+	expectedRepos := []string{"goose", "sprinkler", "slacker"}
+	if len(socialChannel.Repos) != len(expectedRepos) {
+		t.Errorf("expected %d repos in social channel, got %d", len(expectedRepos), len(socialChannel.Repos))
+	}
+	for i, repo := range expectedRepos {
+		if i >= len(socialChannel.Repos) || socialChannel.Repos[i] != repo {
+			t.Errorf("expected repo %q at index %d in social channel, got %v", repo, i, socialChannel.Repos)
+		}
+	}
+
+	// Verify ReminderDMDelay returns correct value
+	delay := m.ReminderDMDelay("codeGROOVE-dev", "social")
+	if delay != 1 {
+		t.Errorf("expected ReminderDMDelay to return 1 minute, got %d", delay)
+	}
+}
+
 func TestManager_LoadConfigEmptyContent(t *testing.T) {
 	handler := func(w http.ResponseWriter, r *http.Request) {
 		// Return a response with nil Content

pkg/home/fetcher_test.go

@@ -260,6 +260,18 @@ func (m *mockStateStore) Cleanup() error {
 	return nil
 }
 
+func (m *mockStateStore) QueuePendingDM(dm state.PendingDM) error {
+	return nil
+}
+
+func (m *mockStateStore) GetPendingDMs(before time.Time) ([]state.PendingDM, error) {
+	return nil, nil
+}
+
+func (m *mockStateStore) RemovePendingDM(id string) error {
+	return nil
+}
+
 func (m *mockStateStore) Close() error {
 	return nil
 }

pkg/notify/daily_digest_test.go

@@ -603,7 +603,7 @@ func TestNewDailyDigestScheduler_FactoryWorks(t *testing.T) {
 	mockConfigMgr := &mockConfigProvider{}
 	mockState := &mockStateProvider{}
 	mockSlack := &mockSlackManagerWithClient{}
-	manager := New(mockSlack, mockConfigMgr)
+	manager := New(mockSlack, mockConfigMgr, &mockStore{})
 
 	scheduler := NewDailyDigestScheduler(manager, mockGitHubMgr, mockConfigMgr, mockState, mockSlack)