Improve WorkflowState handling

pkg/bot/integration_test.go

@@ -107,7 +107,7 @@ func TestTurnclientEmojiIntegration(t *testing.T) {
 	}{
 		{
 			name:          "newly published takes precedence",
-			workflowState: "newly_published",
+			workflowState: string(turn.StateNewlyPublished),
 			nextActions: map[string]turn.Action{
 				"author": {Kind: turn.ActionFixTests},
 			},
@@ -196,7 +196,7 @@ func TestTurnclientEmojiIntegration(t *testing.T) {
 
 			// Verify consistency: if there's a primary action, its emoji should match
 			// (except for newly_published and merged edge cases)
-			if tt.workflowState != "newly_published" && tt.workflowState != "merged" && tt.expectedAction != "" {
+			if tt.workflowState != string(turn.StateNewlyPublished) && tt.workflowState != "merged" && tt.expectedAction != "" {
 				actionEmoji := notify.PrefixForAction(tt.expectedAction)
 				if emoji != actionEmoji {
 					t.Errorf("%s: emoji mismatch - PrefixForAnalysis=%q but PrefixForAction(%q)=%q",

pkg/notify/format_test.go

@@ -382,7 +382,7 @@ func TestFormatChannelMessageBase(t *testing.T) {
 				CheckResult: &turn.CheckResponse{
 					PullRequest: prx.PullRequest{},
 					Analysis: turn.Analysis{
-						WorkflowState: "newly_published",
+						WorkflowState: string(turn.StateNewlyPublished),
 					},
 				},
 				Owner:    "test-owner",
@@ -395,24 +395,154 @@ func TestFormatChannelMessageBase(t *testing.T) {
 			contains: []string{":new:", "New PR", "test-repo#44", "testuser", "?st=newly_published"},
 		},
 		{
-			name: "with next actions",
+			name: "IN_DRAFT",
 			params: MessageParams{
 				CheckResult: &turn.CheckResponse{
 					PullRequest: prx.PullRequest{},
 					Analysis: turn.Analysis{
-						NextAction: map[string]turn.Action{
-							"user1": {Kind: turn.ActionReview},
-						},
+						WorkflowState: string(turn.StateInDraft),
 					},
 				},
 				Owner:    "test-owner",
 				Repo:     "test-repo",
 				PRNumber: 45,
-				Title:    "PR needs review",
+				Title:    "Draft PR",
 				Author:   "testuser",
 				HTMLURL:  "https://github.com/test-owner/test-repo/pull/45",
 			},
-			contains: []string{":hourglass:", "PR needs review", "test-repo#45", "testuser"},
+			contains: []string{":construction:", "Draft PR", "test-repo#45", "testuser", "?st=draft"},
+		},
+		{
+			name: "PUBLISHED_WAITING_FOR_TESTS with broken tests",
+			params: MessageParams{
+				CheckResult: &turn.CheckResponse{
+					PullRequest: prx.PullRequest{},
+					Analysis: turn.Analysis{
+						WorkflowState: string(turn.StatePublishedWaitingForTests),
+						NextAction: map[string]turn.Action{
+							"author": {Kind: turn.ActionFixTests},
+						},
+					},
+				},
+				Owner:    "test-owner",
+				Repo:     "test-repo",
+				PRNumber: 46,
+				Title:    "PR with broken tests",
+				Author:   "testuser",
+				HTMLURL:  "https://github.com/test-owner/test-repo/pull/46",
+			},
+			contains: []string{":cockroach:", "PR with broken tests", "test-repo#46", "testuser", "?st=tests_broken"},
+		},
+		{
+			name: "PUBLISHED_WAITING_FOR_TESTS with pending tests",
+			params: MessageParams{
+				CheckResult: &turn.CheckResponse{
+					PullRequest: prx.PullRequest{},
+					Analysis: turn.Analysis{
+						WorkflowState: string(turn.StatePublishedWaitingForTests),
+						NextAction: map[string]turn.Action{
+							"author": {Kind: turn.ActionTestsPending},
+						},
+					},
+				},
+				Owner:    "test-owner",
+				Repo:     "test-repo",
+				PRNumber: 47,
+				Title:    "PR with tests running",
+				Author:   "testuser",
+				HTMLURL:  "https://github.com/test-owner/test-repo/pull/47",
+			},
+			contains: []string{":test_tube:", "PR with tests running", "test-repo#47", "testuser", "?st=tests_running"},
+		},
+		{
+			name: "TESTED_WAITING_FOR_ASSIGNMENT",
+			params: MessageParams{
+				CheckResult: &turn.CheckResponse{
+					PullRequest: prx.PullRequest{},
+					Analysis: turn.Analysis{
+						WorkflowState: string(turn.StateTestedWaitingForAssignment),
+					},
+				},
+				Owner:    "test-owner",
+				Repo:     "test-repo",
+				PRNumber: 48,
+				Title:    "PR needs reviewer assignment",
+				Author:   "testuser",
+				HTMLURL:  "https://github.com/test-owner/test-repo/pull/48",
+			},
+			contains: []string{":shrug:", "PR needs reviewer assignment", "test-repo#48", "testuser", "?st=awaiting_assignment"},
+		},
+		{
+			name: "ASSIGNED_WAITING_FOR_REVIEW",
+			params: MessageParams{
+				CheckResult: &turn.CheckResponse{
+					PullRequest: prx.PullRequest{},
+					Analysis: turn.Analysis{
+						WorkflowState: string(turn.StateAssignedWaitingForReview),
+					},
+				},
+				Owner:    "test-owner",
+				Repo:     "test-repo",
+				PRNumber: 49,
+				Title:    "PR awaiting review",
+				Author:   "testuser",
+				HTMLURL:  "https://github.com/test-owner/test-repo/pull/49",
+			},
+			contains: []string{":hourglass:", "PR awaiting review", "test-repo#49", "testuser", "?st=awaiting_review"},
+		},
+		{
+			name: "REVIEWED_NEEDS_REFINEMENT",
+			params: MessageParams{
+				CheckResult: &turn.CheckResponse{
+					PullRequest: prx.PullRequest{},
+					Analysis: turn.Analysis{
+						WorkflowState: string(turn.StateReviewedNeedsRefinement),
+					},
+				},
+				Owner:    "test-owner",
+				Repo:     "test-repo",
+				PRNumber: 50,
+				Title:    "PR needs changes",
+				Author:   "testuser",
+				HTMLURL:  "https://github.com/test-owner/test-repo/pull/50",
+			},
+			contains: []string{":carpentry_saw:", "PR needs changes", "test-repo#50", "testuser", "?st=changes_requested"},
+		},
+		{
+			name: "REFINED_WAITING_FOR_APPROVAL",
+			params: MessageParams{
+				CheckResult: &turn.CheckResponse{
+					PullRequest: prx.PullRequest{},
+					Analysis: turn.Analysis{
+						WorkflowState: string(turn.StateRefinedWaitingForApproval),
+					},
+				},
+				Owner:    "test-owner",
+				Repo:     "test-repo",
+				PRNumber: 51,
+				Title:    "PR awaiting approval",
+				Author:   "testuser",
+				HTMLURL:  "https://github.com/test-owner/test-repo/pull/51",
+			},
+			contains: []string{":hourglass:", "PR awaiting approval", "test-repo#51", "testuser", "?st=awaiting_approval"},
+		},
+		{
+			name: "APPROVED_WAITING_FOR_MERGE",
+			params: MessageParams{
+				CheckResult: &turn.CheckResponse{
+					PullRequest: prx.PullRequest{},
+					Analysis: turn.Analysis{
+						WorkflowState: string(turn.StateApprovedWaitingForMerge),
+					},
+				},
+				Owner:    "test-owner",
+				Repo:     "test-repo",
+				PRNumber: 52,
+				Title:    "PR approved and ready",
+				Author:   "testuser",
+				HTMLURL:  "https://github.com/test-owner/test-repo/pull/52",
+			},
+			contains: []string{":white_check_mark:", "PR approved and ready", "test-repo#52", "testuser", "?st=approved"},
 		},
 	}
 

pkg/notify/notify.go

@@ -76,15 +76,18 @@ func FormatChannelMessageBase(ctx context.Context, params MessageParams) string
 	} 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 a.WorkflowState != "" {
+		// Use WorkflowState as primary signal (most reliable source of truth)
+		emoji, state = emojiFromWorkflowState(a.WorkflowState, a.NextAction)
+		slog.Info("using emoji from workflow_state", "pr", prID, "workflow_state", a.WorkflowState, "emoji", emoji, "state_param", state)
 	} else if len(a.NextAction) > 0 {
+		// Fallback to NextAction if no WorkflowState (shouldn't normally happen)
 		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)
+		slog.Info("using emoji from primary next_action (no workflow_state)", "pr", prID, "primary_action", action, "emoji", emoji, "state_param", state)
 	} else {
+		// Final fallback based on PR properties
 		emoji, state = fallbackEmoji(params.CheckResult)
 		//nolint:revive // line length acceptable for structured logging
 		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")
@@ -114,6 +117,62 @@ func FormatNextActionsSuffix(ctx context.Context, params MessageParams) string {
 	return ""
 }
 
+// emojiFromWorkflowState determines emoji based on WorkflowState as the primary signal.
+// Uses NextAction for additional granularity in specific states (e.g., test failures).
+func emojiFromWorkflowState(workflowState string, nextActions map[string]turn.Action) (emoji, state string) {
+	switch workflowState {
+	case string(turn.StateNewlyPublished):
+		return ":new:", "?st=newly_published"
+
+	case string(turn.StateInDraft):
+		return ":construction:", "?st=draft"
+
+	case string(turn.StatePublishedWaitingForTests):
+		// Use NextAction to distinguish between broken tests vs pending tests
+		if len(nextActions) > 0 {
+			action := PrimaryAction(nextActions)
+			if action == string(turn.ActionFixTests) {
+				return ":cockroach:", "?st=tests_broken"
+			}
+			if action == string(turn.ActionTestsPending) {
+				return ":test_tube:", "?st=tests_running"
+			}
+		}
+		// Default to tests running
+		return ":test_tube:", "?st=tests_running"
+
+	case string(turn.StateTestedWaitingForAssignment):
+		// Waiting for reviewers to be assigned
+		return ":shrug:", "?st=awaiting_assignment"
+
+	case string(turn.StateAssignedWaitingForReview):
+		// Waiting for assigned reviewers to review
+		return ":hourglass:", "?st=awaiting_review"
+
+	case string(turn.StateReviewedNeedsRefinement):
+		// Author needs to address feedback/comments
+		return ":carpentry_saw:", "?st=changes_requested"
+
+	case string(turn.StateRefinedWaitingForApproval):
+		// Waiting for final approval after refinements
+		return ":hourglass:", "?st=awaiting_approval"
+
+	case string(turn.StateApprovedWaitingForMerge):
+		// Approved and ready to merge
+		return ":white_check_mark:", "?st=approved"
+
+	default:
+		// Unknown workflow state - fall back to NextAction
+		slog.Warn("unknown workflow state, falling back to NextAction",
+			"workflow_state", workflowState)
+		if len(nextActions) > 0 {
+			action := PrimaryAction(nextActions)
+			return PrefixForAction(action), "?st=unknown"
+		}
+		return ":postal_horn:", "?st=unknown"
+	}
+}
+
 // stateParam returns the URL state parameter based on PR analysis.
 func stateParam(r *turn.CheckResponse) string {
 	pr := r.PullRequest
@@ -376,8 +435,8 @@ func PrimaryAction(nextActions map[string]turn.Action) string {
 
 // 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.
+// 1. Use WorkflowState as primary signal (most reliable)
+// 2. Fall back to NextAction if no WorkflowState
 func PrefixForAnalysis(workflowState string, nextActions map[string]turn.Action) string {
 	// Log input for debugging emoji selection
 	actionKinds := make([]string, 0, len(nextActions))
@@ -389,25 +448,27 @@ func PrefixForAnalysis(workflowState string, nextActions map[string]turn.Action)
 		"next_actions_count", len(nextActions),
 		"next_actions", actionKinds)
 
-	// Special case: newly published PRs always show :new:
-	if workflowState == "newly_published" {
-		slog.Debug("using :new: emoji for newly published PR")
-		return ":new:"
+	// Use WorkflowState as primary signal
+	if workflowState != "" {
+		emoji, _ := emojiFromWorkflowState(workflowState, nextActions)
+		slog.Debug("determined emoji from workflow_state",
+			"workflow_state", workflowState,
+			"emoji", emoji)
+		return emoji
 	}
 
-	// Determine primary action and return corresponding emoji
+	// Fallback to NextAction if no WorkflowState
 	primaryAction := PrimaryAction(nextActions)
 	if primaryAction != "" {
 		emoji := PrefixForAction(primaryAction)
-		slog.Debug("determined emoji from primary action",
+		slog.Debug("determined emoji from primary action (no workflow_state)",
 			"primary_action", primaryAction,
 			"emoji", emoji)
 		return emoji
 	}
 
-	// Fallback if no actions
-	slog.Info("no primary action found - using fallback emoji",
-		"workflow_state", workflowState,
+	// Final fallback if no workflow state or actions
+	slog.Info("no workflow_state or primary action - using fallback emoji",
 		"next_actions_count", len(nextActions),
 		"fallback_emoji", ":postal_horn:")
 	return ":postal_horn:"