channel DM message refactor

go.mod

@@ -7,8 +7,8 @@ require (
 	github.com/codeGROOVE-dev/gh-mailto v0.0.0-20251024133418-149270eb16a9
 	github.com/codeGROOVE-dev/gsm v0.0.0-20251019065141-833fe2363d22
 	github.com/codeGROOVE-dev/retry v1.3.0
-	github.com/codeGROOVE-dev/sprinkler v0.0.0-20251027213037-05bb80a9db89
-	github.com/codeGROOVE-dev/turnclient v0.0.0-20251022064427-5a712e1e10e6
+	github.com/codeGROOVE-dev/sprinkler v0.0.0-20251028184624-4d8c8315a53a
+	github.com/codeGROOVE-dev/turnclient v0.0.0-20251028130307-1f85c9aa43c4
 	github.com/golang-jwt/jwt/v5 v5.3.0
 	github.com/google/go-github/v50 v50.2.0
 	github.com/gorilla/mux v1.8.1
@@ -47,7 +47,7 @@ require (
 	golang.org/x/net v0.46.0 // indirect
 	golang.org/x/sys v0.37.0 // indirect
 	golang.org/x/text v0.30.0 // indirect
-	google.golang.org/api v0.253.0 // indirect
+	google.golang.org/api v0.254.0 // indirect
 	google.golang.org/genproto v0.0.0-20251022142026-3a174f9686a8 // indirect
 	google.golang.org/genproto/googleapis/api v0.0.0-20251022142026-3a174f9686a8 // indirect
 	google.golang.org/genproto/googleapis/rpc v0.0.0-20251022142026-3a174f9686a8 // indirect

go.sum

@@ -33,8 +33,12 @@ github.com/codeGROOVE-dev/sprinkler v0.0.0-20251027150242-98f387d0502a h1:bMRtyB
 github.com/codeGROOVE-dev/sprinkler v0.0.0-20251027150242-98f387d0502a/go.mod h1:/kd3ncsRNldD0MUpbtp5ojIzfCkyeXB7JdOrpuqG7Gg=
 github.com/codeGROOVE-dev/sprinkler v0.0.0-20251027213037-05bb80a9db89 h1:8Z3SM90hy1nuK2r2yhtv4HwitnO9si4GzVRktRDQ68g=
 github.com/codeGROOVE-dev/sprinkler v0.0.0-20251027213037-05bb80a9db89/go.mod h1:/kd3ncsRNldD0MUpbtp5ojIzfCkyeXB7JdOrpuqG7Gg=
+github.com/codeGROOVE-dev/sprinkler v0.0.0-20251028184624-4d8c8315a53a h1:P1qUAC4QG4zNH31y0R4ZJdnqpqHM+chKrTiI86GlIaw=
+github.com/codeGROOVE-dev/sprinkler v0.0.0-20251028184624-4d8c8315a53a/go.mod h1:/kd3ncsRNldD0MUpbtp5ojIzfCkyeXB7JdOrpuqG7Gg=
 github.com/codeGROOVE-dev/turnclient v0.0.0-20251022064427-5a712e1e10e6 h1:7FCmaftkl362oTZHVJyUg+xhxqfQFx+JisBf7RgklL8=
 github.com/codeGROOVE-dev/turnclient v0.0.0-20251022064427-5a712e1e10e6/go.mod h1:fYwtN9Ql6lY8t2WvCfENx+mP5FUwjlqwXCLx9CVLY20=
+github.com/codeGROOVE-dev/turnclient v0.0.0-20251028130307-1f85c9aa43c4 h1:si9tMEo5SXpDuDXGkJ1zNnnpP8TbmakrkNujAbpKlqA=
+github.com/codeGROOVE-dev/turnclient v0.0.0-20251028130307-1f85c9aa43c4/go.mod h1:bFWMd0JeaJY0kSIO5AcRQdJLXF3Fo3eKclE49vmIZes=
 github.com/davecgh/go-spew v1.1.1 h1:vj9j/u1bqnvCEfJOwUhtlOARqs3+rkHYY13jYWTU97c=
 github.com/davecgh/go-spew v1.1.1/go.mod h1:J7Y8YcW2NihsgmVo/mv3lAwl/skON4iLHjSsI+c5H38=
 github.com/envoyproxy/go-control-plane v0.13.4 h1:zEqyPVyku6IvWCFwux4x9RxkLOMUL+1vC9xUFv5l2/M=
@@ -127,6 +131,8 @@ gonum.org/v1/gonum v0.16.0 h1:5+ul4Swaf3ESvrOnidPp4GZbzf0mxVQpDCYUQE7OJfk=
 gonum.org/v1/gonum v0.16.0/go.mod h1:fef3am4MQ93R2HHpKnLk4/Tbh/s0+wqD5nfa6Pnwy4E=
 google.golang.org/api v0.253.0 h1:apU86Eq9Q2eQco3NsUYFpVTfy7DwemojL7LmbAj7g/I=
 google.golang.org/api v0.253.0/go.mod h1:PX09ad0r/4du83vZVAaGg7OaeyGnaUmT/CYPNvtLCbw=
+google.golang.org/api v0.254.0 h1:jl3XrGj7lRjnlUvZAbAdhINTLbsg5dbjmR90+pTQvt4=
+google.golang.org/api v0.254.0/go.mod h1:5BkSURm3D9kAqjGvBNgf0EcbX6Rnrf6UArKkwBzAyqQ=
 google.golang.org/genproto v0.0.0-20251022142026-3a174f9686a8 h1:a12a2/BiVRxRWIqBbfqoSK6tgq8cyUgMnEI81QlPge0=
 google.golang.org/genproto v0.0.0-20251022142026-3a174f9686a8/go.mod h1:1Ic78BnpzY8OaTCmzxJDP4qC9INZPbGZl+54RKjtyeI=
 google.golang.org/genproto/googleapis/api v0.0.0-20251022142026-3a174f9686a8 h1:mepRgnBZa07I4TRuomDE4sTIYieg/osKmzIf4USdWS4=

internal/bot/bot.go

@@ -1375,37 +1375,19 @@ func (c *Coordinator) processPRForChannel(
 	// Build what the message SHOULD be based on current PR state
 	// Then compare to what it IS - update if different
 	if !wasNewlyCreated {
-		// Determine expected prefix based on workflow state and next actions
-		var expectedPrefix string
-		if checkResult != nil {
-			slog.Info("determining message update emoji from analysis",
-				"pr", fmt.Sprintf("%s/%s#%d", owner, repo, prNumber),
-				"workflow_state", checkResult.Analysis.WorkflowState,
-				"next_action_count", len(checkResult.Analysis.NextAction),
-				"pr_state_fallback", prState)
-			expectedPrefix = notify.PrefixForAnalysis(checkResult.Analysis.WorkflowState, checkResult.Analysis.NextAction)
-		} else {
-			slog.Info("no analysis available - using state-based emoji fallback for message update",
-				"pr", fmt.Sprintf("%s/%s#%d", owner, repo, prNumber),
-				"pr_state", prState)
-			expectedPrefix = notify.PrefixForState(prState)
-		}
 		domain := c.configManager.Domain(owner)
-		urlWithState := event.PullRequest.HTMLURL + c.getStateQueryParam(prState)
-
-		expectedText := fmt.Sprintf("%s %s <%s|%s#%d> · %s",
-			expectedPrefix,
-			event.PullRequest.Title,
-			urlWithState,
-			repo,
-			prNumber,
-			event.PullRequest.User.Login,
-		)
-
-		nextActions := c.formatNextActions(ctx, checkResult, owner, domain)
-		if nextActions != "" {
-			expectedText += fmt.Sprintf(" → %s", nextActions)
+		params := notify.MessageParams{
+			CheckResult: checkResult,
+			Owner:       owner,
+			Repo:        repo,
+			PRNumber:    prNumber,
+			Title:       event.PullRequest.Title,
+			Author:      event.PullRequest.User.Login,
+			HTMLURL:     event.PullRequest.HTMLURL,
+			Domain:      domain,
+			UserMapper:  c.userMapper,
 		}
+		expectedText := notify.FormatChannelMessageBase(ctx, params) + notify.FormatNextActionsSuffix(ctx, params)
 
 		// Compare expected vs actual - update if different
 		if currentText != expectedText {
@@ -1629,36 +1611,21 @@ func (c *Coordinator) createPRThread(ctx context.Context, channel, owner, repo s
 	Number  int    `json:"number"`
 }, checkResult *turn.CheckResponse,
 ) (threadTS string, messageText string, err error) {
-	// Get emoji prefix based on workflow state and next actions
-	var prefix string
-	if checkResult != nil {
-		slog.Info("determining new thread emoji from analysis",
-			"pr", fmt.Sprintf("%s/%s#%d", owner, repo, number),
-			"workflow_state", checkResult.Analysis.WorkflowState,
-			"next_action_count", len(checkResult.Analysis.NextAction),
-			"pr_state_fallback", prState)
-		prefix = notify.PrefixForAnalysis(checkResult.Analysis.WorkflowState, checkResult.Analysis.NextAction)
-	} else {
-		// Fallback to state-based prefix if no checkResult available
-		slog.Info("no analysis available - using state-based emoji fallback for new thread",
-			"pr", fmt.Sprintf("%s/%s#%d", owner, repo, number),
-			"pr_state", prState)
-		prefix = notify.PrefixForState(prState)
-	}
-
-	// Add state query param to URL for debugging
-	urlWithState := pr.HTMLURL + c.getStateQueryParam(prState)
-
 	// Format initial message WITHOUT user mentions (fast path)
 	// Format: :emoji: Title repo#123 · author
-	initialText := fmt.Sprintf("%s %s <%s|%s#%d> · %s",
-		prefix,
-		pr.Title,
-		urlWithState,
-		repo,
-		number,
-		pr.User.Login,
-	)
+	domain := c.configManager.Domain(owner)
+	params := notify.MessageParams{
+		CheckResult: checkResult,
+		Owner:       owner,
+		Repo:        repo,
+		PRNumber:    number,
+		Title:       pr.Title,
+		Author:      pr.User.Login,
+		HTMLURL:     pr.HTMLURL,
+		Domain:      domain,
+		UserMapper:  c.userMapper,
+	}
+	initialText := notify.FormatChannelMessageBase(ctx, params)
 
 	// Resolve channel name to ID for consistent API calls
 	resolvedChannel := c.slack.ResolveChannelID(ctx, channel)
@@ -1694,7 +1661,6 @@ func (c *Coordinator) createPRThread(ctx context.Context, channel, owner, repo s
 
 	// Asynchronously add user mentions once email lookups complete
 	// This avoids blocking thread creation on slow email lookups (13-20 seconds each)
-	domain := c.configManager.Domain(owner)
 	if checkResult != nil && len(checkResult.Analysis.NextAction) > 0 {
 		// SECURITY NOTE: Use detached context to complete message enrichment even during shutdown.
 		// Operations bounded by 60-second timeout, allowing time for:
@@ -1710,14 +1676,15 @@ func (c *Coordinator) createPRThread(ctx context.Context, channel, owner, repo s
 		capturedNumber := number
 		capturedChannel := resolvedChannel
 		capturedInitialText := initialText
+		capturedParams := params
 		go func() {
 			defer enrichCancel()
 
 			// Perform email lookups in background
-			nextActions := c.formatNextActions(enrichCtx, checkResult, capturedOwner, domain)
-			if nextActions != "" {
+			nextActionsSuffix := notify.FormatNextActionsSuffix(enrichCtx, capturedParams)
+			if nextActionsSuffix != "" {
 				// Update message with user mentions
-				enrichedText := capturedInitialText + fmt.Sprintf(" → %s", nextActions)
+				enrichedText := capturedInitialText + nextActionsSuffix
 				if err := c.slack.UpdateMessage(enrichCtx, capturedChannel, capturedThreadTS, enrichedText); err != nil {
 					slog.Warn("failed to update thread with user mentions (async enrichment)",
 						logFieldPR, fmt.Sprintf(prFormatString, capturedOwner, capturedRepo, capturedNumber),
@@ -1729,7 +1696,7 @@ func (c *Coordinator) createPRThread(ctx context.Context, channel, owner, repo s
 						logFieldPR, fmt.Sprintf(prFormatString, capturedOwner, capturedRepo, capturedNumber),
 						logFieldChannel, capturedChannel,
 						"thread_ts", capturedThreadTS,
-						"next_actions", nextActions)
+						"next_actions_suffix", nextActionsSuffix)
 				}
 			}
 		}()

internal/notify/notify.go

@@ -5,6 +5,7 @@ import (
 	"context"
 	"fmt"
 	"log/slog"
+	"strings"
 	"time"
 
 	"github.com/codeGROOVE-dev/slacker/internal/slack"
@@ -16,6 +17,187 @@ const (
 	defaultReminderDMDelayMinutes = 65 // Default delay in minutes before sending DM if user tagged in channel
 )
 
+// UserMapper interface for formatting user mentions in messages.
+type UserMapper interface {
+	FormatUserMentions(ctx context.Context, githubUsers []string, owner, domain string) string
+}
+
+// MessageParams contains all parameters needed to format a channel message.
+type MessageParams struct {
+	CheckResult *turn.CheckResponse
+	Owner       string
+	Repo        string
+	PRNumber    int
+	Title       string
+	Author      string
+	HTMLURL     string
+	Domain      string
+	UserMapper  UserMapper
+}
+
+// FormatChannelMessageBase formats the base Slack channel message for a PR (without next actions).
+// This is the single source of truth for channel message formatting.
+// It handles emoji determination and base message assembly.
+// Use FormatNextActionsSuffix to add next actions when needed.
+func FormatChannelMessageBase(ctx context.Context, params MessageParams) string {
+	pr := params.CheckResult.PullRequest
+	a := params.CheckResult.Analysis
+	prID := fmt.Sprintf("%s/%s#%d", params.Owner, params.Repo, params.PRNumber)
+
+	// Log all decision factors for debugging
+	kinds := make([]string, 0, len(a.NextAction))
+	for user, action := range a.NextAction {
+		kinds = append(kinds, fmt.Sprintf("%s:%s", user, action.Kind))
+	}
+
+	slog.Info("formatting channel message",
+		"pr", prID,
+		"pr_state", pr.State,
+		"merged", pr.Merged,
+		"draft", pr.Draft,
+		"workflow_state", a.WorkflowState,
+		"next_action_count", len(a.NextAction),
+		"next_actions", kinds,
+		"checks_failing", a.Checks.Failing,
+		"checks_pending", a.Checks.Pending,
+		"checks_waiting", a.Checks.Waiting,
+		"approved", a.Approved,
+		"unresolved_comments", a.UnresolvedComments)
+
+	// Determine emoji and state parameter
+	var emoji, state string
+
+	// Handle merged/closed states first (most definitive)
+	if pr.Merged {
+		emoji, state = ":rocket:", "?st=merged"
+		slog.Info("using :rocket: emoji - PR is merged", "pr", prID, "merged_at", pr.MergedAt)
+	} else if pr.State == "closed" {
+		emoji, state = ":x:", "?st=closed"
+		slog.Info("using :x: emoji - PR is closed but not merged", "pr", prID)
+	} else if a.WorkflowState == "newly_published" {
+		emoji, state = ":new:", "?st=newly_published"
+		slog.Info("using :new: emoji - newly published PR", "pr", prID, "workflow_state", a.WorkflowState)
+	} else if len(a.NextAction) > 0 {
+		action := PrimaryAction(a.NextAction)
+		emoji = PrefixForAction(action)
+		state = stateParam(params.CheckResult)
+		slog.Info("using emoji from primary next_action", "pr", prID, "primary_action", action, "emoji", emoji, "state_param", state)
+	} else {
+		emoji, state = fallbackEmoji(params.CheckResult)
+		slog.Info("using fallback emoji - no workflow_state or next_actions", "pr", prID, "emoji", emoji, "state_param", state, "fallback_reason", "empty_workflow_state_and_next_actions")
+	}
+
+	return fmt.Sprintf("%s %s <%s|%s#%d> · %s",
+		emoji,
+		params.Title,
+		params.HTMLURL+state,
+		params.Repo,
+		params.PRNumber,
+		params.Author)
+}
+
+// FormatNextActionsSuffix formats the next actions suffix for a channel message.
+// Returns empty string if no next actions are present.
+// Call this after FormatChannelMessageBase to build a complete message with user mentions.
+func FormatNextActionsSuffix(ctx context.Context, params MessageParams) string {
+	if params.CheckResult == nil || len(params.CheckResult.Analysis.NextAction) == 0 {
+		return ""
+	}
+
+	actions := formatNextActionsInternal(ctx, params.CheckResult.Analysis.NextAction, params.Owner, params.Domain, params.UserMapper)
+	if actions != "" {
+		return fmt.Sprintf(" → %s", actions)
+	}
+	return ""
+}
+
+// stateParam returns the URL state parameter based on PR analysis.
+func stateParam(r *turn.CheckResponse) string {
+	pr := r.PullRequest
+	a := r.Analysis
+
+	if pr.Draft {
+		return "?st=tests_running"
+	}
+	if a.Checks.Failing > 0 {
+		return "?st=tests_broken"
+	}
+	if a.Checks.Pending > 0 || a.Checks.Waiting > 0 {
+		return "?st=tests_running"
+	}
+	if a.Approved {
+		if a.UnresolvedComments > 0 {
+			return "?st=changes_requested"
+		}
+		return "?st=approved"
+	}
+	return "?st=awaiting_review"
+}
+
+// fallbackEmoji determines emoji when no workflow_state or next_actions are available.
+func fallbackEmoji(r *turn.CheckResponse) (emoji, state string) {
+	pr := r.PullRequest
+	a := r.Analysis
+
+	if pr.Draft {
+		return ":test_tube:", "?st=tests_running"
+	}
+	if a.Checks.Failing > 0 {
+		return ":cockroach:", "?st=tests_broken"
+	}
+	if a.Checks.Pending > 0 || a.Checks.Waiting > 0 {
+		return ":test_tube:", "?st=tests_running"
+	}
+	if a.Approved {
+		if a.UnresolvedComments > 0 {
+			return ":carpentry_saw:", "?st=changes_requested"
+		}
+		return ":white_check_mark:", "?st=approved"
+	}
+	return ":hourglass:", "?st=awaiting_review"
+}
+
+// formatNextActionsInternal formats next actions with user mentions (internal helper).
+func formatNextActionsInternal(ctx context.Context, nextActions map[string]turn.Action, owner, domain string, userMapper UserMapper) string {
+	if len(nextActions) == 0 {
+		return ""
+	}
+
+	// Group users by action kind, filtering out _system users
+	actionGroups := make(map[string][]string)
+	for user, action := range nextActions {
+		actionKind := string(action.Kind)
+		// Skip _system users but still track the action exists
+		if user != "_system" {
+			actionGroups[actionKind] = append(actionGroups[actionKind], user)
+		} else if _, exists := actionGroups[actionKind]; !exists {
+			// Action only has _system - create empty slice to track it exists
+			actionGroups[actionKind] = []string{}
+		}
+	}
+
+	// Format each action group
+	var parts []string
+	for actionKind, users := range actionGroups {
+		// Convert snake_case to space-separated words
+		actionName := strings.ReplaceAll(actionKind, "_", " ")
+
+		// Format user mentions (will be empty if only _system was assigned)
+		userMentions := userMapper.FormatUserMentions(ctx, users, owner, domain)
+
+		// If action has users, format as "action: users"
+		// If no users (was only _system), just show the action
+		if userMentions != "" {
+			parts = append(parts, fmt.Sprintf("%s: %s", actionName, userMentions))
+		} else {
+			parts = append(parts, actionName)
+		}
+	}
+
+	// Use semicolons to separate different actions (commas are used between users)
+	return strings.Join(parts, "; ")
+}
+
 // Manager handles user notifications across multiple workspaces.
 type Manager struct {
 	slackManager  *slack.Manager

internal/notify/tracker_test.go

@@ -291,13 +291,13 @@ func TestNotificationTracker_KeyFormats(t *testing.T) {
 
 	// Test that various special characters in keys work correctly
 	tests := []struct {
-		name       string
-		workspace  string
-		user       string
-		owner      string
-		repo       string
-		prNumber   int
-		channelID  string
+		name      string
+		workspace string
+		user      string
+		owner     string
+		repo      string
+		prNumber  int
+		channelID string
 	}{
 		{
 			name:      "simple keys",