ctx, lint

Author: Thomas Stromberg

.golangci.yml

@@ -70,6 +70,7 @@ linters:
     - testableexamples # checks if examples are testable (have an expected output)
     - testpackage # makes you use a separate _test package
     - paralleltest # not every test should be in parallel
+    - tparallel # table-driven tests share mock servers for performance
     - wrapcheck # not required
     - wsl # [too strict and mostly code is not more readable] whitespace linter forces you to use empty lines
     - wsl_v5 # [too strict and mostly code is not more readable] add or remove empty lines

cmd/server/main.go

@@ -249,8 +249,8 @@ func run(ctx context.Context, cancel context.CancelFunc, cfg *config.ServerConfi
 	// Get GitHub token from one of the installations
 	var githubToken string
 	for _, org := range githubManager.AllOrgs() {
-		if client, ok := githubManager.ClientForOrg(org); ok {
-			githubToken = client.InstallationToken(ctx)
+		if ghClient, ok := githubManager.ClientForOrg(org); ok {
+			githubToken = ghClient.InstallationToken(ctx)
 			break
 		}
 	}
@@ -731,7 +731,7 @@ func runBotCoordinators(
 
 	// Run cleanup once on startup
 	go func() {
-		if err := stateStore.Cleanup(); err != nil {
+		if err := stateStore.Cleanup(context.Background()); err != nil {
 			slog.Warn("initial state cleanup failed", "error", err)
 		}
 	}()
@@ -765,7 +765,7 @@ func runBotCoordinators(
 		case <-cleanupTicker.C:
 			// Periodic cleanup of old state data
 			go func() {
-				if err := stateStore.Cleanup(); err != nil {
+				if err := stateStore.Cleanup(context.Background()); err != nil {
 					slog.Warn("state cleanup failed", "error", err)
 				} else {
 					slog.Debug("state cleanup completed successfully")

pkg/bot/bot.go

@@ -57,15 +57,15 @@ type Coordinator struct {
 
 // StateStore interface for persistent state - allows dependency injection for testing.
 type StateStore interface {
-	Thread(owner, repo string, number int, channelID string) (cache.ThreadInfo, bool)
-	SaveThread(owner, repo string, number int, channelID string, info cache.ThreadInfo) error
-	LastDM(userID, prURL string) (time.Time, bool)
-	RecordDM(userID, prURL string, sentAt time.Time) error
-	ListDMUsers(prURL string) []string
-	WasProcessed(eventKey string) bool
-	MarkProcessed(eventKey string, ttl time.Duration) error
-	LastNotification(prURL string) time.Time
-	RecordNotification(prURL string, notifiedAt time.Time) error
+	Thread(ctx context.Context, owner, repo string, number int, channelID string) (cache.ThreadInfo, bool)
+	SaveThread(ctx context.Context, owner, repo string, number int, channelID string, info cache.ThreadInfo) error
+	LastDM(ctx context.Context, userID, prURL string) (time.Time, bool)
+	RecordDM(ctx context.Context, userID, prURL string, sentAt time.Time) error
+	ListDMUsers(ctx context.Context, prURL string) []string
+	WasProcessed(ctx context.Context, eventKey string) bool
+	MarkProcessed(ctx context.Context, eventKey string, ttl time.Duration) error
+	LastNotification(ctx context.Context, prURL string) time.Time
+	RecordNotification(ctx context.Context, prURL string, notifiedAt time.Time) error
 	Close() error
 }
 
@@ -118,13 +118,13 @@ func New(
 
 // saveThread persists thread info to both cache and persistent storage.
 // This ensures threads survive restarts and are available for closed PR updates.
-func (c *Coordinator) saveThread(owner, repo string, number int, channelID string, info cache.ThreadInfo) {
+func (c *Coordinator) saveThread(ctx context.Context, owner, repo string, number int, channelID string, info cache.ThreadInfo) {
 	// Save to in-memory cache for fast lookups
 	key := fmt.Sprintf("%s/%s#%d:%s", owner, repo, number, channelID)
 	c.threadCache.Set(key, info)
 
 	// Persist to state store for cross-instance sharing and restart recovery
-	if err := c.stateStore.SaveThread(owner, repo, number, channelID, info); err != nil {
+	if err := c.stateStore.SaveThread(ctx, owner, repo, number, channelID, info); err != nil {
 		slog.Warn("failed to persist thread to state store",
 			"pr", fmt.Sprintf("%s/%s#%d", owner, repo, number),
 			"channel_id", channelID,
@@ -196,7 +196,7 @@ func (c *Coordinator) findOrCreatePRThread(ctx context.Context, channelID, owner
 			"current_message_preview", initialSearchText[:min(100, len(initialSearchText))])
 
 		// Save the found thread (cache + persist)
-		c.saveThread(owner, repo, prNumber, channelID, cache.ThreadInfo{
+		c.saveThread(ctx, owner, repo, prNumber, channelID, cache.ThreadInfo{
 			ThreadTS:    initialSearchTS,
 			ChannelID:   channelID,
 			LastState:   prState,
@@ -272,7 +272,7 @@ func (c *Coordinator) findOrCreatePRThread(ctx context.Context, channelID, owner
 			"note", "this prevented duplicate thread creation during rolling deployment")
 
 		// Save it and return (cache + persist)
-		c.saveThread(owner, repo, prNumber, channelID, cache.ThreadInfo{
+		c.saveThread(ctx, owner, repo, prNumber, channelID, cache.ThreadInfo{
 			ThreadTS:    crossInstanceCheckTS,
 			ChannelID:   channelID,
 			LastState:   prState,
@@ -295,7 +295,7 @@ func (c *Coordinator) findOrCreatePRThread(ctx context.Context, channelID, owner
 	}
 
 	// Save the new thread (cache + persist)
-	c.saveThread(owner, repo, prNumber, channelID, cache.ThreadInfo{
+	c.saveThread(ctx, owner, repo, prNumber, channelID, cache.ThreadInfo{
 		ThreadTS:    newThreadTS,
 		ChannelID:   channelID,
 		LastState:   prState,
@@ -925,7 +925,7 @@ func (c *Coordinator) updateDMMessagesForPR(ctx context.Context, pr prUpdateInfo
 
 	// For terminal states (merged/closed), update all users who received DMs
 	if prState == "merged" || prState == "closed" {
-		slackUserIDs = c.stateStore.ListDMUsers(prURL)
+		slackUserIDs = c.stateStore.ListDMUsers(ctx, prURL)
 		if len(slackUserIDs) == 0 {
 			slog.Debug("no DM recipients found for merged/closed PR",
 				"pr", fmt.Sprintf("%s/%s#%d", owner, repo, prNumber),
@@ -983,8 +983,7 @@ func (c *Coordinator) updateDMMessagesForPR(ctx context.Context, pr prUpdateInfo
 		slog.Info("no analysis available - using state-based emoji fallback",
 			"pr", fmt.Sprintf("%s/%s#%d", owner, repo, prNumber),
 			"pr_state", prState)
-		//nolint:staticcheck // deprecated method kept for backward compatibility
-		prefix = notify.PrefixForState(prState)
+		prefix = notify.PrefixForAnalysis("", nil)
 	}
 	var action string
 	switch prState {
@@ -1216,6 +1215,8 @@ func (c *Coordinator) processChannelsInParallel(
 
 // processPRForChannel handles PR processing for a single channel (extracted from the main loop).
 // Returns a map of Slack user IDs that were successfully tagged in this channel.
+//
+//nolint:maintidx // Core PR processing logic with necessary complexity for handling notifications
 func (c *Coordinator) processPRForChannel(
 	ctx context.Context, prCtx prContext, channelName, workspaceID string,
 ) map[string]bool {
@@ -1385,7 +1386,7 @@ func (c *Coordinator) processPRForChannel(
 					"next_poll_in", "5m")
 			} else {
 				// Save updated thread info (cache + persist)
-				c.saveThread(owner, repo, prNumber, channelID, cache.ThreadInfo{
+				c.saveThread(ctx, owner, repo, prNumber, channelID, cache.ThreadInfo{
 					ThreadTS:    threadTS,
 					ChannelID:   channelID,
 					LastState:   prState,

pkg/bot/bot_sprinkler.go

@@ -118,6 +118,7 @@ func (c *Coordinator) lookupPRsForCheckEvent(ctx context.Context, event client.E
 	// If yes, fetch it via turnclient to see if it contains this commit
 	// This is cheaper than searching all PRs via GitHub API
 	mostRecentPR := c.commitPRCache.MostRecentPR(owner, repo)
+	//nolint:nestif // Complex but necessary cache population logic with early returns
 	if mostRecentPR > 0 {
 		slog.Debug("attempting turnclient lookup on most recent PR for repo",
 			"organization", organization,
@@ -156,14 +157,14 @@ func (c *Coordinator) lookupPRsForCheckEvent(ctx context.Context, event client.E
 
 							// Populate cache with all commits from this PR
 							for _, commit := range checkResult.PullRequest.Commits {
+								//nolint:revive // Nesting depth acceptable for cache population logic
 								if commit != "" {
 									c.commitPRCache.RecordPR(owner, repo, mostRecentPR, commit)
 								}
 							}
 
 							// Process the PR update since we have fresh data
-							//nolint:contextcheck // Background context intentional - goroutine must outlive parent timeout
-							go c.handlePullRequestEventWithData(context.Background(), owner, repo, struct {
+							c.handlePullRequestEventWithData(ctx, owner, repo, struct {
 								Action      string `json:"action"`
 								PullRequest struct {
 									HTMLURL   string    `json:"html_url"`
@@ -270,7 +271,7 @@ func (c *Coordinator) handleSprinklerEvent(ctx context.Context, event client.Eve
 
 	// Try to claim this event atomically using persistent store (Datastore transaction).
 	// This is the single source of truth for cross-instance deduplication.
-	if err := c.stateStore.MarkProcessed(eventKey, 24*time.Hour); err != nil {
+	if err := c.stateStore.MarkProcessed(ctx, eventKey, 24*time.Hour); err != nil {
 		// Check if this is a race condition vs a database error
 		if errors.Is(err, state.ErrAlreadyProcessed) {
 			slog.Info("skipping duplicate event - claimed by this or another instance",

pkg/bot/bot_test.go

@@ -167,6 +167,7 @@ func TestNew_WithGitHubClient(t *testing.T) {
 }
 
 func TestSaveThread(t *testing.T) {
+	ctx := context.Background()
 	mockSlack := &mockSlackClient{}
 	configMgr := NewMockConfig().Build()
 
@@ -190,7 +191,7 @@ func TestSaveThread(t *testing.T) {
 		ThreadTS:  "1234567890.123456",
 	}
 
-	c.saveThread("testorg", "testrepo", 42, "C123456", threadInfo)
+	c.saveThread(ctx, "testorg", "testrepo", 42, "C123456", threadInfo)
 
 	// Check cache
 	key := "testorg/testrepo#42:C123456"
@@ -220,6 +221,7 @@ func TestSaveThread(t *testing.T) {
 }
 
 func TestSaveThread_PersistenceError(t *testing.T) {
+	ctx := context.Background()
 	mockSlack := &mockSlackClient{}
 	configMgr := NewMockConfig().Build()
 
@@ -245,7 +247,7 @@ func TestSaveThread_PersistenceError(t *testing.T) {
 	}
 
 	// Should still save to cache even if persistence fails
-	c.saveThread("testorg", "testrepo", 42, "C123456", threadInfo)
+	c.saveThread(ctx, "testorg", "testrepo", 42, "C123456", threadInfo)
 
 	// Check cache (should succeed)
 	key := "testorg/testrepo#42:C123456"

pkg/bot/cache/commit_pr.go

@@ -7,17 +7,17 @@ import (
 
 // CommitPREntry caches recent commit→PR mappings for fast lookup.
 type CommitPREntry struct {
-	PRNumber  int
-	HeadSHA   string
 	UpdatedAt time.Time
+	HeadSHA   string
+	PRNumber  int
 }
 
 // CommitPRCache provides in-memory caching of commit SHA → PR mappings.
 // This allows quick lookup when check events arrive with just a commit SHA,
 // avoiding expensive GitHub API calls for recently-seen PRs.
 type CommitPRCache struct {
+	entries map[string][]CommitPREntry
 	mu      sync.RWMutex
-	entries map[string][]CommitPREntry // "owner/repo" -> recent PRs with commits
 }
 
 // NewCommitPRCache creates a new CommitPRCache with initialized maps.

pkg/bot/cache/commit_pr_test.go

@@ -266,20 +266,20 @@ func TestCommitPRCache_Concurrency(t *testing.T) {
 
 	// Concurrent writes
 	done := make(chan bool)
-	for i := 0; i < 10; i++ {
+	for i := range 10 {
 		go func(prNum int) {
 			cache.RecordPR("owner", "repo", prNum, "commit"+string(rune(prNum)))
 			done <- true
 		}(i)
 	}
 
 	// Wait for all writes
-	for i := 0; i < 10; i++ {
+	for range 10 {
 		<-done
 	}
 
 	// Concurrent reads
-	for i := 0; i < 10; i++ {
+	for i := range 10 {
 		go func(prNum int) {
 			_ = cache.FindPRsForCommit("owner", "repo", "commit"+string(rune(prNum)))
 			_ = cache.MostRecentPR("owner", "repo")
@@ -288,7 +288,7 @@ func TestCommitPRCache_Concurrency(t *testing.T) {
 	}
 
 	// Wait for all reads
-	for i := 0; i < 10; i++ {
+	for range 10 {
 		<-done
 	}
 

pkg/bot/cache/thread.go

@@ -1,3 +1,4 @@
+// Package cache provides thread information caching for bot operations.
 package cache
 
 import (

pkg/bot/cache/thread_test.go

@@ -318,7 +318,7 @@ func TestThreadCache_Concurrency(t *testing.T) {
 
 	// Concurrent operations on different keys
 	done := make(chan bool)
-	for i := 0; i < 10; i++ {
+	for i := range 10 {
 		go func(n int) {
 			key := "key" + string(rune(n))
 			info := ThreadInfo{ThreadTS: "123.456", ChannelID: "C123"}
@@ -334,13 +334,13 @@ func TestThreadCache_Concurrency(t *testing.T) {
 	}
 
 	// Wait for all goroutines
-	for i := 0; i < 10; i++ {
+	for range 10 {
 		<-done
 	}
 
 	// Concurrent operations on same key
 	key := "shared"
-	for i := 0; i < 10; i++ {
+	for range 10 {
 		go func() {
 			info := ThreadInfo{ThreadTS: "123.456", ChannelID: "C123"}
 			cache.Set(key, info)
@@ -349,21 +349,21 @@ func TestThreadCache_Concurrency(t *testing.T) {
 		}()
 	}
 
-	for i := 0; i < 10; i++ {
+	for range 10 {
 		<-done
 	}
 
 	// Concurrent MarkCreating on same key (only first should succeed)
 	successCount := 0
 	resultChan := make(chan bool, 10)
 
-	for i := 0; i < 10; i++ {
+	for range 10 {
 		go func() {
 			resultChan <- cache.MarkCreating("concurrent-test")
 		}()
 	}
 
-	for i := 0; i < 10; i++ {
+	for range 10 {
 		if <-resultChan {
 			successCount++
 		}
@@ -374,14 +374,14 @@ func TestThreadCache_Concurrency(t *testing.T) {
 	}
 
 	// Cleanup concurrency test
-	for i := 0; i < 5; i++ {
+	for range 5 {
 		go func() {
 			cache.Cleanup(1 * time.Hour)
 			done <- true
 		}()
 	}
 
-	for i := 0; i < 5; i++ {
+	for range 5 {
 		<-done
 	}
 

pkg/bot/coordinator_test.go

@@ -1,13 +1,15 @@
 package bot
 
 import (
+	"context"
 	"testing"
 	"time"
 
 	"github.com/codeGROOVE-dev/slacker/pkg/bot/cache"
 )
 
 func TestCoordinator_saveThread(t *testing.T) {
+	ctx := context.Background()
 	// Create mock state store
 	mockStore := &mockStateStore{
 		threads: make(map[string]cache.ThreadInfo),
@@ -33,7 +35,7 @@ func TestCoordinator_saveThread(t *testing.T) {
 		LastEventTime: time.Now(),
 	}
 
-	c.saveThread(owner, repo, number, channelID, info)
+	c.saveThread(ctx, owner, repo, number, channelID, info)
 
 	// Verify thread was saved to cache
 	cacheKey := owner + "/" + repo + "#123:" + channelID