Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions .claude/CLAUDE.md
Original file line number Diff line number Diff line change
Expand Up @@ -139,6 +139,7 @@ slacker/
- `reactions:write` - Add and edit emoji reactions
- `team:read` - **View workspace name and domain (required for workspace validation)**
- `users:read` - View people in the workspace
- `users:read.email` - **Access user email addresses (required for GitHub→Slack user mapping)**

### State Management
- Track PR states and transitions for notification logic
Expand Down
34 changes: 29 additions & 5 deletions internal/bot/bot.go
Original file line number Diff line number Diff line change
Expand Up @@ -654,7 +654,7 @@ func (c *Coordinator) handlePullRequestEventWithData(ctx context.Context, owner,
dmCtx, dmCancel := context.WithTimeout(context.WithoutCancel(ctx), 15*time.Second)
go func() {
defer dmCancel()
c.sendDMNotifications(dmCtx, workspaceID, owner, repo, prNumber, uniqueUsers, event, prState)
c.sendDMNotifications(dmCtx, workspaceID, owner, repo, prNumber, uniqueUsers, event, prState, checkResult)
}()
} else {
slog.Info("no users blocking PR - no notifications needed",
Expand Down Expand Up @@ -682,6 +682,7 @@ func (c *Coordinator) sendDMNotifications(
Number int `json:"number"`
},
prState string,
checkResult *turn.CheckResponse,
) {
slog.Info("starting DM notification batch",
logFieldPR, fmt.Sprintf(prFormatString, owner, repo, prNumber),
Expand Down Expand Up @@ -726,6 +727,11 @@ func (c *Coordinator) sendDMNotifications(
State: prState,
HTMLURL: event.PullRequest.HTMLURL,
}
// Add workflow state and next actions if available
if checkResult != nil {
prInfo.WorkflowState = checkResult.Analysis.WorkflowState
prInfo.NextAction = checkResult.Analysis.NextAction
}

err = c.notifier.NotifyUser(ctx, workspaceID, slackUserID, tagInfo.ChannelID, channelName, prInfo)
if err != nil {
Expand Down Expand Up @@ -895,7 +901,13 @@ func (c *Coordinator) updateDMMessagesForPR(ctx context.Context, pr prUpdateInfo
}

// Format the DM message (same format as initial send)
prefix := notify.PrefixForState(prState)
// Determine prefix based on workflow state and next actions
var prefix string
if checkResult != nil {
prefix = notify.PrefixForAnalysis(checkResult.Analysis.WorkflowState, checkResult.Analysis.NextAction)
} else {
prefix = notify.PrefixForState(prState)
}
var action string
switch prState {
case "newly_published":
Expand Down Expand Up @@ -1232,7 +1244,13 @@ 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 {
expectedPrefix := notify.PrefixForState(prState)
// Determine expected prefix based on workflow state and next actions
var expectedPrefix string
if checkResult != nil {
expectedPrefix = notify.PrefixForAnalysis(checkResult.Analysis.WorkflowState, checkResult.Analysis.NextAction)
} else {
expectedPrefix = notify.PrefixForState(prState)
}
domain := c.configManager.Domain(owner)
urlWithState := event.PullRequest.HTMLURL + c.getStateQueryParam(prState)

Expand Down Expand Up @@ -1469,8 +1487,14 @@ 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 state-based prefix
prefix := notify.PrefixForState(prState)
// Get emoji prefix based on workflow state and next actions
var prefix string
if checkResult != nil {
prefix = notify.PrefixForAnalysis(checkResult.Analysis.WorkflowState, checkResult.Analysis.NextAction)
} else {
// Fallback to state-based prefix if no checkResult available
prefix = notify.PrefixForState(prState)
}

// Add state query param to URL for debugging
urlWithState := pr.HTMLURL + c.getStateQueryParam(prState)
Expand Down
118 changes: 109 additions & 9 deletions internal/notify/notify.go
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@ import (
"time"

"github.com/codeGROOVE-dev/slacker/internal/slack"
"github.com/codeGROOVE-dev/turnclient/pkg/turn"
)

// Constants for notification defaults.
Expand Down Expand Up @@ -77,17 +78,21 @@ func (m *Manager) Run(ctx context.Context) error {

// PRInfo contains the minimal information needed to notify about a PR.
type PRInfo struct {
Owner string
Repo string
Title string
Author string
State string
HTMLURL string
Number int
Owner string
Repo string
Title string
Author string
State string // Deprecated: use WorkflowState and NextAction for emoji determination
HTMLURL string
Number int
WorkflowState string // Workflow state from turnclient Analysis
NextAction map[string]turn.Action // Next actions from turnclient Analysis
}

// PrefixForState returns the emoji prefix for a given PR state.
// Exported for use by bot package to ensure consistent PR state display.
//
// Deprecated: Use PrefixForAnalysis instead to derive emoji from NextAction.
func PrefixForState(prState string) string {
switch prState {
case "newly_published":
Expand All @@ -111,6 +116,95 @@ func PrefixForState(prState string) string {
}
}

// PrefixForAction returns the emoji prefix for a given action kind.
func PrefixForAction(action string) string {
switch action {
case "publish_draft":
return ":construction:"
case "fix_tests":
return ":cockroach:"
case "tests_pending":
return ":test_tube:"
case "review", "re_review", "request_reviewers":
return ":hourglass:"
case "resolve_comments", "respond", "review_discussion":
return ":carpentry_saw:"
case "approve":
return ":white_check_mark:"
case "merge":
return ":rocket:"
default:
return ":postal_horn:"
}
}

// actionPriority returns a priority score for action kinds (lower = higher priority).
// Used to determine which emoji to show when multiple actions are pending.
func actionPriority(action string) int {
switch action {
case "publish_draft":
return 1
case "fix_tests":
return 2
case "tests_pending":
return 3
case "fix_conflict":
return 4
case "resolve_comments", "respond", "review_discussion":
return 5
case "review", "re_review", "request_reviewers":
return 6
case "approve":
return 7
case "merge":
return 8
default:
return 99
}
}

// PrimaryAction determines the primary action kind from a NextAction map.
// Returns the highest-priority action (lowest priority score).
func PrimaryAction(nextActions map[string]turn.Action) string {
if len(nextActions) == 0 {
return ""
}

var primaryAction string
minPriority := 999

for _, action := range nextActions {
kind := string(action.Kind)
priority := actionPriority(kind)
if priority < minPriority {
minPriority = priority
primaryAction = kind
}
}

return primaryAction
}

// PrefixForAnalysis returns the emoji prefix based on workflow state and next actions.
// This is the primary function for determining PR emoji - it handles the logic:
// 1. If workflow_state == "newly_published" → ":new:"
// 2. Otherwise → emoji based on primary next_action
func PrefixForAnalysis(workflowState string, nextActions map[string]turn.Action) string {
// Special case: newly published PRs always show :new:
if workflowState == "newly_published" {
return ":new:"
}

// Determine primary action and return corresponding emoji
primaryAction := PrimaryAction(nextActions)
if primaryAction != "" {
return PrefixForAction(primaryAction)
}

// Fallback if no actions
return ":postal_horn:"
}

// NotifyUser sends a smart notification to a user about a PR using the configured logic.
// Implements delayed DM logic: if user was tagged in channel, delay by configured time.
// If user is not in channel where tagged, send DM immediately.
Expand Down Expand Up @@ -245,8 +339,14 @@ func (m *Manager) NotifyUser(ctx context.Context, workspaceID, userID, channelID
}

// Format notification message using same style as channel messages
// Use state-based emoji prefix like channel messages do
prefix := PrefixForState(pr.State)
// Determine emoji prefix based on workflow state and next actions
var prefix string
if pr.WorkflowState != "" {
prefix = PrefixForAnalysis(pr.WorkflowState, pr.NextAction)
} else {
// Fallback to state if workflow state not available
prefix = PrefixForState(pr.State)
}

// Format: :emoji: Title <url|repo#123> · author → action
var action string
Expand Down
85 changes: 72 additions & 13 deletions internal/usermapping/usermapping.go
Original file line number Diff line number Diff line change
Expand Up @@ -138,10 +138,16 @@ func (s *Service) SlackHandle(ctx context.Context, githubUsername, organization,
// Extracted from SlackHandle to work with singleflight pattern.
func (s *Service) doLookup(ctx context.Context, githubUsername, organization, domain string) (string, error) {
// Get emails for GitHub user with organization context
slog.Info("starting email lookup for GitHub user",
"github_user", githubUsername,
"organization", organization,
"domain", domain)

result, err := s.githubLookup.Lookup(ctx, githubUsername, organization)
if err != nil {
slog.Warn("failed to get emails for GitHub user",
"github_user", githubUsername,
"organization", organization,
"error", err)
return "", err
}
Expand All @@ -154,10 +160,17 @@ func (s *Service) doLookup(ctx context.Context, githubUsername, organization, do
emails[i] = addr.Email
}

slog.Debug("found emails for GitHub user",
slog.Info("found emails for GitHub user via lookup",
"github_user", githubUsername,
"email_count", len(emails),
"emails", emails)
"emails", emails,
"methods", func() []string {
methods := make([]string, len(result.Addresses))
for i, addr := range result.Addresses {
methods[i] = addr.Methods[0] // Show first method
}
return methods
}())

// Try finding Slack matches with all emails first
matches := s.findSlackMatches(ctx, githubUsername, emails)
Expand Down Expand Up @@ -208,28 +221,38 @@ func (s *Service) doLookup(ctx context.Context, githubUsername, organization, do
}
}
// Finally, try intelligent guessing
slog.Debug("trying intelligent email guessing",
slog.Info("starting intelligent email guessing",
"github_user", githubUsername,
"domain", domain)
"organization", organization,
"domain", domain,
"found_addresses_count", len(result.Addresses))

guessResult, err := s.githubLookup.Guess(ctx, githubUsername, organization, ghmailto.GuessOptions{
Domain: domain,
})
if err != nil {
slog.Warn("email guessing failed",
"github_user", githubUsername,
"organization", organization,
"domain", domain,
"error", err)
} else if len(guessResult.Guesses) > 0 {
guessedEmails := make([]string, len(guessResult.Guesses))
confidences := make([]int, len(guessResult.Guesses))
patterns := make([]string, len(guessResult.Guesses))
for i, addr := range guessResult.Guesses {
guessedEmails[i] = addr.Email
confidences[i] = addr.Confidence
patterns[i] = addr.Pattern
}

slog.Debug("generated email guesses",
slog.Info("generated email guesses for Slack lookup",
"github_user", githubUsername,
"domain", domain,
"guesses", guessedEmails)
"guess_count", len(guessedEmails),
"guesses", guessedEmails,
"confidences", confidences,
"patterns", patterns)

matches := s.findSlackMatches(ctx, githubUsername, guessedEmails)
if len(matches) > 0 {
Expand All @@ -243,13 +266,25 @@ func (s *Service) doLookup(ctx context.Context, githubUsername, organization, do
s.cacheMapping(bestMatch)
return bestMatch.SlackUserID, nil
}
slog.Warn("email guesses generated but no Slack matches found",
"github_user", githubUsername,
"domain", domain,
"guesses_tried", len(guessedEmails),
"guesses", guessedEmails)
} else {
slog.Warn("email guessing completed but no guesses generated",
"github_user", githubUsername,
"domain", domain)
}
}

// No matches found through any method
slog.Info("no Slack mapping found for GitHub user",
slog.Warn("no Slack mapping found for GitHub user after exhausting all methods",
"github_user", githubUsername,
"organization", organization,
"domain", domain,
"tried_direct_emails", len(emails) > 0,
"direct_emails_tried", emails,
"tried_domain_filtering", domain != "",
"tried_guessing", domain != "")

Expand Down Expand Up @@ -387,15 +422,31 @@ func (s *Service) cacheMapping(mapping *UserMapping) {
func (s *Service) findSlackMatches(ctx context.Context, githubUsername string, emails []string) []*UserMapping {
var matches []*UserMapping

slog.Info("starting Slack user lookup phase",
"github_user", githubUsername,
"email_count", len(emails),
"emails", emails)

// Search for each email in Slack user directory
for _, email := range emails {
for i, email := range emails {
// Normalize email to lowercase for consistent matching
normalizedEmail := strings.ToLower(email)
slog.Info("attempting Slack API lookup",
"github_user", githubUsername,
"attempt", i+1,
"of", len(emails),
"original_email", email,
"normalized_email", normalizedEmail)

user, err := s.slackClient.GetUserByEmailContext(ctx, normalizedEmail)
if err != nil {
slog.Debug("no Slack user found for email",
// Log detailed error info to help debug Slack API issues
slog.Warn("Slack API lookup failed for email",
"github_user", githubUsername,
"email", email,
"error", err)
"normalized_email", normalizedEmail,
"error_type", fmt.Sprintf("%T", err),
"error_message", err.Error())
continue
}

Expand All @@ -420,12 +471,20 @@ func (s *Service) findSlackMatches(ctx context.Context, githubUsername string, e

matches = append(matches, mapping)

slog.Debug("found Slack user match",
slog.Info("found Slack user match",
"github_user", githubUsername,
"email", email,
"slack_user", user.Name,
"confidence", confidence)
"slack_user_id", user.ID,
"slack_user_name", user.Name,
"confidence", confidence,
"is_primary_email", user.Profile.Email == email)
}

slog.Info("Slack user lookup phase complete",
"github_user", githubUsername,
"emails_tried", len(emails),
"matches_found", len(matches))

return matches
}

Expand Down