From 8797667fac42091abc4f70542017cd80bcf88b40 Mon Sep 17 00:00:00 2001 From: ZeroIce <39822906+vicksiyi@users.noreply.github.com> Date: Fri, 3 Jul 2026 12:33:35 +0800 Subject: [PATCH] fix(slack): prefer attachment body over fallback (#4866) Co-authored-by: multica-agent --- server/internal/integrations/slack/history.go | 18 +++---- .../integrations/slack/history_test.go | 48 ++++++++++++++++++- 2 files changed, 56 insertions(+), 10 deletions(-) diff --git a/server/internal/integrations/slack/history.go b/server/internal/integrations/slack/history.go index 9aecafeaf4..6e8373f133 100644 --- a/server/internal/integrations/slack/history.go +++ b/server/internal/integrations/slack/history.go @@ -283,8 +283,9 @@ const maxDerivedTextLen = 4000 // Block Kit blocks and leave the top-level Text empty; without this fallback // such a message is indistinguishable from a join/system marker and gets // dropped (MUL-3931 / #4803). Order: top-level text, then each attachment's -// built-in Fallback / title+text/fields, then a best-effort blocks flatten. -// Returns "" only when nothing renderable exists — a real system marker. +// rendered text/fields, then last-resort fallback text, then a best-effort +// blocks flatten. Returns "" only when nothing renderable exists — a real +// system marker. func flattenSlackText(m slack.Message) string { if t := strings.TrimSpace(m.Text); t != "" { return t @@ -303,13 +304,11 @@ func flattenSlackText(m slack.Message) string { return truncateRunes(strings.TrimSpace(strings.Join(parts, "\n")), maxDerivedTextLen) } -// attachmentText summarizes one attachment, preferring Slack's built-in -// Fallback (the plain-text rendering Slack ships for exactly this purpose), -// then a pretext/title/text/fields composite, then the attachment's own blocks. +// attachmentText summarizes one attachment. Attachment fallback is only a +// last-resort summary for clients that cannot render attachments; Grafana-style +// alerts often put the useful alert body in Text/Fields while Fallback repeats +// the short title. func attachmentText(a slack.Attachment) string { - if t := strings.TrimSpace(a.Fallback); t != "" { - return t - } parts := make([]string, 0, 3+len(a.Fields)) for _, s := range []string{a.Pretext, a.Title, a.Text} { if s = strings.TrimSpace(s); s != "" { @@ -324,6 +323,9 @@ func attachmentText(a slack.Attachment) string { if len(parts) > 0 { return strings.Join(parts, "\n") } + if t := strings.TrimSpace(a.Fallback); t != "" { + return t + } return flattenBlocks(a.Blocks) } diff --git a/server/internal/integrations/slack/history_test.go b/server/internal/integrations/slack/history_test.go index 7b921a165f..571cb0f6e4 100644 --- a/server/internal/integrations/slack/history_test.go +++ b/server/internal/integrations/slack/history_test.go @@ -279,14 +279,58 @@ func TestThreadRecoversBotAttachmentText(t *testing.T) { if got == nil { t.Fatalf("bot root message was dropped: %+v", page.Messages) } - if got.Text != "[FIRING:1] HighLatency prod" { - t.Errorf("root text = %q, want the attachment fallback", got.Text) + if got.Text != "HighLatency\np99 over threshold" { + t.Errorf("root text = %q, want the attachment title and body", got.Text) } if got.Author != "Grafana" { t.Errorf("root author = %q, want Grafana", got.Author) } } +func TestAttachmentTextPriority(t *testing.T) { + tests := []struct { + name string + att slack.Attachment + want string + }{ + { + name: "fallback only", + att: slack.Attachment{ + Fallback: "short fallback", + }, + want: "short fallback", + }, + { + name: "text beats fallback", + att: slack.Attachment{ + Fallback: "[FIRING:1] HighLatency prod", + Title: "HighLatency", + Text: "Summary: p99 over threshold\nDescription: checkout is slow", + }, + want: "HighLatency\nSummary: p99 over threshold\nDescription: checkout is slow", + }, + { + name: "fields beat fallback", + att: slack.Attachment{ + Fallback: "short fallback", + Fields: []slack.AttachmentField{ + {Title: "severity", Value: "critical"}, + {Title: "pod", Value: "checkout-7d8"}, + }, + }, + want: "severity critical\npod checkout-7d8", + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + if got := attachmentText(tt.att); got != tt.want { + t.Fatalf("attachmentText() = %q, want %q", got, tt.want) + } + }) + } +} + // TestThreadRecoversBlocksText: a message with no Text and no attachment // fallback is flattened from its Block Kit blocks. func TestThreadRecoversBlocksText(t *testing.T) {