feat(slack): ask for a thumbs up/down on completion replies in pilot channels

cmd/kelos-slack-server/main.go

@@ -126,10 +126,11 @@ func main() {
 	// activity loops. Sharing the instance ensures activity state is
 	// correctly cleared when a progress snapshot is posted.
 	slackReporter := &reporting.SlackTaskReporter{
-		Client:         mgr.GetClient(),
-		Reporter:       &reporting.SlackReporter{BotToken: botToken},
-		ProgressReader: &reporting.DefaultProgressReader{Clientset: clientset},
-		ActivityReader: &reporting.DefaultActivityReader{Clientset: clientset},
+		Client:             mgr.GetClient(),
+		Reporter:           &reporting.SlackReporter{BotToken: botToken},
+		ProgressReader:     &reporting.DefaultProgressReader{Clientset: clientset},
+		ActivityReader:     &reporting.DefaultActivityReader{Clientset: clientset},
+		ScorePilotChannels: reporting.ParseScorePilotChannels(os.Getenv("SLACK_SCORE_PILOT_CHANNELS")),
 	}
 
 	// Register reporting loop as a leader-elected runnable.

internal/manifests/charts/kelos/templates/slack-server.yaml

@@ -51,6 +51,10 @@ spec:
             - name: SLACK_JOIN_MESSAGE_FILE
               value: /etc/kelos/join-message.txt
             {{- end }}
+            {{- if .Values.slackServer.scorePilotChannels }}
+            - name: SLACK_SCORE_PILOT_CHANNELS
+              value: {{ join "," .Values.slackServer.scorePilotChannels | quote }}
+            {{- end }}
           ports:
             - name: metrics
               containerPort: 8080

internal/manifests/charts/kelos/values.yaml

@@ -195,6 +195,9 @@ slackServer:
   joinMessage: ""
   # Deny service to externally shared (Slack Connect) channels.
   denySlackConnectChannels: false
+  # Channel IDs whose completion replies carry the 👍/👎 score ask and
+  # pre-seeded reactions. Leave empty to disable the score ask everywhere.
+  scorePilotChannels: []
   resources:
     limits:
       cpu: 500m

internal/reporting/slack.go

@@ -5,6 +5,7 @@ import (
 	"encoding/base64"
 	"errors"
 	"fmt"
+	"strings"
 	"sync"
 	"unicode/utf8"
 
@@ -79,6 +80,37 @@ func (r *SlackReporter) UpdateMessage(ctx context.Context, channel, messageTS st
 	return nil
 }
 
+// AddReaction adds a reaction emoji to a posted message. Used to pre-seed
+// the 👍 and 👎 score reactions on completion replies in the pilot channels.
+func (r *SlackReporter) AddReaction(ctx context.Context, channel, messageTS, emoji string) error {
+	item := slack.NewRefToMessage(channel, messageTS)
+	if err := r.api().AddReactionContext(ctx, emoji, item); err != nil {
+		return fmt.Errorf("adding Slack reaction %s to message %s: %w", emoji, messageTS, err)
+	}
+	return nil
+}
+
+// ParseScorePilotChannels splits a comma-separated channel-ID list (e.g.
+// the SLACK_SCORE_PILOT_CHANNELS environment variable) into a set. Empty
+// parts and stray whitespace are ignored, so "C1, ,C2 " and "C1,C2" are
+// equivalent. A blank input yields nil, which keeps the score ask disabled
+// everywhere.
+func ParseScorePilotChannels(value string) map[string]bool {
+	if strings.TrimSpace(value) == "" {
+		return nil
+	}
+	channels := map[string]bool{}
+	for _, part := range strings.Split(value, ",") {
+		if channel := strings.TrimSpace(part); channel != "" {
+			channels[channel] = true
+		}
+	}
+	if len(channels) == 0 {
+		return nil
+	}
+	return channels
+}
+
 // isPermanentSlackError reports whether err is a Slack API error that will
 // never succeed if retried, such as replying to a message Slack does not
 // allow replies to ("cannot_reply_to_message"). HTTP-level failures
@@ -107,6 +139,19 @@ func contextBlock(taskName string) *slack.ContextBlock {
 	)
 }
 
+// scoreAskText invites the human to rate the completion reply with a
+// one-tap 👍/👎 reaction. It appears only on terminal ("succeeded" or
+// "failed") transition replies in the score pilot channels, where both
+// reactions are pre-seeded so tapping is all it takes.
+const scoreAskText = "React 👍/👎 to score this."
+
+// scoreAskBlock returns the context block rendering the score ask.
+func scoreAskBlock() *slack.ContextBlock {
+	return slack.NewContextBlock("",
+		slack.NewTextBlockObject(slack.MarkdownType, scoreAskText, false, false),
+	)
+}
+
 // phaseHeaderText maps each phase to its leading Block Kit section text.
 // Phases without an entry (e.g. "succeeded") get no header block.
 var phaseHeaderText = map[string]string{
@@ -146,6 +191,15 @@ func FormatProgressMessage(text, taskName string) SlackMessage {
 // so that no individual message exceeds the Slack block limit, and each chunk
 // is posted as a separate thread reply.
 func FormatSlackTransitionMessage(phase, taskName, message string, results map[string]string) []SlackMessage {
+	return formatSlackTransitionMessage(phase, taskName, message, results, false)
+}
+
+// formatSlackTransitionMessage is the shared implementation of
+// FormatSlackTransitionMessage. When askForScore is true and the phase is
+// terminal, the trailing blocks additionally carry the 👍/👎 score ask
+// (scoreAskBlock) just above the Task: footer, and the pre-seeded reactions
+// make it a one-tap verdict.
+func formatSlackTransitionMessage(phase, taskName, message string, results map[string]string, askForScore bool) []SlackMessage {
 	// Build the optional header block (e.g. "Working on your request…").
 	var headerBlocks []slack.Block
 	if header, ok := phaseHeaderText[phase]; ok {
@@ -163,7 +217,7 @@ func FormatSlackTransitionMessage(phase, taskName, message string, results map[s
 		responseBlocks = responseToBlocks(decoded)
 	}
 
-	// Build the trailing blocks (PR link, error, context).
+	// Build the trailing blocks (PR link, error, score ask, context).
 	var trailingBlocks []slack.Block
 	pr := results["pr"]
 	if pr != "" {
@@ -180,6 +234,10 @@ func FormatSlackTransitionMessage(phase, taskName, message string, results map[s
 		))
 	}
 
+	if askForScore && (phase == "succeeded" || phase == "failed") {
+		trailingBlocks = append(trailingBlocks, scoreAskBlock())
+	}
+
 	trailingBlocks = append(trailingBlocks, contextBlock(taskName))
 
 	fallbackText := buildFallbackText(decoded, pr, message, phase, taskName)

internal/reporting/slack_test.go

@@ -11,6 +11,90 @@ import (
 	"github.com/slack-go/slack"
 )
 
+func TestFormatSlackTransitionMessage_ScoreAsk(t *testing.T) {
+	t.Run("succeeded with ask", func(t *testing.T) {
+		got := firstMsg(t, formatSlackTransitionMessage("succeeded", "spawner-1234567890.123456", "", nil, true))
+		// context only: score ask + Task footer
+		assertBlockCount(t, got.Blocks, 2)
+		assertContextContains(t, got.Blocks[0], scoreAskText)
+		assertContextContains(t, got.Blocks[1], "spawner-1234567890.123456")
+	})
+
+	t.Run("failed with ask", func(t *testing.T) {
+		got := firstMsg(t, formatSlackTransitionMessage("failed", "spawner-1234567890.123456", "pod OOMKilled", nil, true))
+		assertBlockCount(t, got.Blocks, 4) // header + error + score ask + Task footer
+		assertContextContains(t, got.Blocks[2], scoreAskText)
+		assertContextContains(t, got.Blocks[3], "spawner-1234567890.123456")
+	})
+
+	t.Run("accepted never asks", func(t *testing.T) {
+		got := firstMsg(t, formatSlackTransitionMessage("accepted", "spawner-1234567890.123456", "", nil, true))
+		assertBlockCount(t, got.Blocks, 2) // header + context, no ask
+		assertContextContains(t, got.Blocks[1], "spawner-1234567890.123456")
+		if blocksContainText(got.Blocks, scoreAskText) {
+			t.Error("accepted message must not carry the score ask")
+		}
+	})
+
+	t.Run("succeeded without ask", func(t *testing.T) {
+		got := firstMsg(t, formatSlackTransitionMessage("succeeded", "spawner-1234567890.123456", "", nil, false))
+		assertBlockCount(t, got.Blocks, 1) // context only, no ask
+		if blocksContainText(got.Blocks, scoreAskText) {
+			t.Error("score ask must only appear when explicitly requested")
+		}
+	})
+
+	t.Run("ask survives splitting on last part", func(t *testing.T) {
+		var sb strings.Builder
+		for i := 0; i < 30; i++ {
+			if i > 0 {
+				sb.WriteString("\n\n")
+			}
+			sb.WriteString("### Header\nSome content here.")
+		}
+		results := map[string]string{"response": b64(sb.String())}
+		msgs := formatSlackTransitionMessage("succeeded", "test-task", "", results, true)
+		if len(msgs) < 2 {
+			t.Fatalf("expected the long response to split, got %d messages", len(msgs))
+		}
+		// Trailing blocks (score ask, then Task footer) ride on the last part.
+		last := msgs[len(msgs)-1]
+		assertContextContains(t, last.Blocks[len(last.Blocks)-2], scoreAskText)
+		assertContextContains(t, last.Blocks[len(last.Blocks)-1], "test-task")
+	})
+}
+
+func TestParseScorePilotChannels(t *testing.T) {
+	tests := []struct {
+		name  string
+		value string
+		want  map[string]bool
+	}{
+		{"empty", "", nil},
+		{"whitespace only", "  ", nil},
+		{"stray commas", ", ,", nil},
+		{"single", "C123ABC", map[string]bool{"C123ABC": true}},
+		{"multiple", "C123ABC,C456DEF", map[string]bool{"C123ABC": true, "C456DEF": true}},
+		{"trims whitespace", " C123ABC , C456DEF ", map[string]bool{"C123ABC": true, "C456DEF": true}},
+		{"drops empty parts", "C123ABC,,C456DEF", map[string]bool{"C123ABC": true, "C456DEF": true}},
+		{"deduplicates", "C123ABC,C123ABC", map[string]bool{"C123ABC": true}},
+	}
+
+	for _, tt := range tests {
+		t.Run(tt.name, func(t *testing.T) {
+			got := ParseScorePilotChannels(tt.value)
+			if len(got) != len(tt.want) {
+				t.Fatalf("ParseScorePilotChannels(%q) = %v, want %v", tt.value, got, tt.want)
+			}
+			for ch := range tt.want {
+				if !got[ch] {
+					t.Errorf("ParseScorePilotChannels(%q) missing %q", tt.value, ch)
+				}
+			}
+		})
+	}
+}
+
 // firstMsg is a helper that returns the first (and usually only) message
 // from FormatSlackTransitionMessage, failing the test if the slice is empty.
 func firstMsg(t *testing.T, msgs []SlackMessage) SlackMessage {

internal/reporting/watcher.go

@@ -473,10 +473,12 @@ func (tr *TaskReporter) persistAnnotations(ctx context.Context, task *kelos.Task
 	return nil
 }
 
-// SlackMessenger is the interface for posting and updating Slack messages.
+// SlackMessenger is the interface for posting, updating, and reacting to
+// Slack messages.
 type SlackMessenger interface {
 	PostThreadReply(ctx context.Context, channel, threadTS string, msg SlackMessage) (string, error)
 	UpdateMessage(ctx context.Context, channel, messageTS string, msg SlackMessage) error
+	AddReaction(ctx context.Context, channel, messageTS, emoji string) error
 }
 
 // activityState tracks the target message for activity indicator updates.
@@ -508,6 +510,11 @@ type SlackTaskReporter struct {
 	ProgressReader ProgressReader
 	ActivityReader ActivityReader
 
+	// ScorePilotChannels is the set of channel IDs whose completion
+	// replies carry the 👍/👎 score ask and pre-seeded reactions. A nil or
+	// empty map disables the ask everywhere.
+	ScorePilotChannels map[string]bool
+
 	mu           sync.Mutex
 	lastProgress map[types.UID]string         // taskUID -> last posted text
 	progressTS   map[types.UID]string         // taskUID -> message ts of the progress reply
@@ -567,7 +574,8 @@ func (tr *SlackTaskReporter) ReportTaskStatus(ctx context.Context, task *kelos.T
 		return nil
 	}
 
-	msgs := FormatSlackTransitionMessage(desiredPhase, task.Name, task.Status.Message, task.Status.Results)
+	askForScore := tr.scoreAskEnabled(channel, desiredPhase)
+	msgs := formatSlackTransitionMessage(desiredPhase, task.Name, task.Status.Message, task.Status.Results, askForScore)
 
 	// For terminal phases, try to edit the existing progress message
 	// in-place. When the response is a single message, this keeps the
@@ -581,11 +589,18 @@ func (tr *SlackTaskReporter) ReportTaskStatus(ctx context.Context, task *kelos.T
 				log.Error(err, "Failed to update progress message with final result, posting new reply", "task", task.Name)
 			} else {
 				// Post any continuation messages as new thread replies.
+				// The last reply carries the trailing blocks, so it is the
+				// one the score ask and pre-seeded reactions belong on.
+				lastReplyTS := progressTS
 				for _, msg := range msgs[1:] {
-					if _, err := tr.Reporter.PostThreadReply(ctx, channel, threadTS, msg); err != nil {
+					replyTS, err := tr.Reporter.PostThreadReply(ctx, channel, threadTS, msg)
+					if err != nil {
 						log.Error(err, "Failed to post continuation message", "task", task.Name)
+						continue
 					}
+					lastReplyTS = replyTS
 				}
+				tr.preSeedScoreReactions(ctx, channel, lastReplyTS, askForScore)
 				tr.clearProgressCache(task.UID)
 				tr.clearActivityState(task.UID)
 				return tr.persistSlackReportingState(ctx, task, desiredPhase)
@@ -595,6 +610,7 @@ func (tr *SlackTaskReporter) ReportTaskStatus(ctx context.Context, task *kelos.T
 
 	// Post all messages as thread replies.
 	var firstReplyTS string
+	var lastReplyTS string
 	for i, msg := range msgs {
 		log.Info("Posting Slack thread reply", "task", task.Name, "channel", channel, "phase", desiredPhase, "part", i+1, "total", len(msgs))
 		replyTS, err := tr.Reporter.PostThreadReply(ctx, channel, threadTS, msg)
@@ -615,8 +631,14 @@ func (tr *SlackTaskReporter) ReportTaskStatus(ctx context.Context, task *kelos.T
 		if i == 0 {
 			firstReplyTS = replyTS
 		}
+		lastReplyTS = replyTS
 	}
 
+	// Pre-seed the 👍/👎 reactions on the completion reply in the pilot
+	// channels so a human taps either one instead of opening the emoji
+	// picker. Skipped when askForScore is false or nothing was posted.
+	tr.preSeedScoreReactions(ctx, channel, lastReplyTS, askForScore)
+
 	// Track the accepted message so the activity loop can update it.
 	if desiredPhase == "accepted" && firstReplyTS != "" {
 		tr.setActivityTarget(task.UID, firstReplyTS, msgs[0])
@@ -635,6 +657,33 @@ func (tr *SlackTaskReporter) ReportTaskStatus(ctx context.Context, task *kelos.T
 	return nil
 }
 
+// scoreAskEnabled reports whether the task's completion reply should carry
+// the 👍/👎 score ask. Only terminal phases get the ask, and only in the
+// pilot channels named in ScorePilotChannels; a nil or empty map disables
+// it everywhere.
+func (tr *SlackTaskReporter) scoreAskEnabled(channel, desiredPhase string) bool {
+	if desiredPhase != "succeeded" && desiredPhase != "failed" {
+		return false
+	}
+	return tr.ScorePilotChannels != nil && tr.ScorePilotChannels[channel]
+}
+
+// preSeedScoreReactions adds the 👍 and 👎 reactions to a posted completion
+// reply so a human taps once instead of opening the emoji picker. The
+// collector ignores bot reactions, so pre-seeding does not skew the verdict
+// ratio. Failures are logged and otherwise ignored: the reply itself is
+// already posted, and a missing reaction only costs the one-tap convenience.
+func (tr *SlackTaskReporter) preSeedScoreReactions(ctx context.Context, channel, messageTS string, askForScore bool) {
+	if !askForScore || messageTS == "" {
+		return
+	}
+	for _, emoji := range []string{"+1", "-1"} {
+		if err := tr.Reporter.AddReaction(ctx, channel, messageTS, emoji); err != nil {
+			ctrl.Log.WithName("slack-reporter").Error(err, "Failed to pre-seed score reaction", "channel", channel, "messageTS", messageTS, "emoji", emoji)
+		}
+	}
+}
+
 func (tr *SlackTaskReporter) persistSlackReportingState(ctx context.Context, task *kelos.Task, desiredPhase string) error {
 	if err := retry.RetryOnConflict(retry.DefaultRetry, func() error {
 		var current kelos.Task