Skip to content
Open
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
15 changes: 15 additions & 0 deletions pkg/channels/channelkinds/slack/blocks_degrade.go
Original file line number Diff line number Diff line change
Expand Up @@ -36,3 +36,18 @@ func isUnsupportedBlocksErr(err error) bool {
return false
}
}

// blocksRejectionDetail returns Slack's per-block explanation for a rejected
// Block Kit payload. Slack puts the offending field(s) in
// ResponseMetadata.Messages (typically a json-pointer into /blocks), but
// SlackErrorResponse.Error() surfaces only the bare code ("invalid_blocks"),
// so without this the specific reason is lost at the degrade sites. Returns nil
// when err is not a typed Slack error or carried no messages — a degrade log
// then simply omits the detail rather than inventing one.
func blocksRejectionDetail(err error) []string {
var sre slackapi.SlackErrorResponse
if !errors.As(err, &sre) {
return nil
}
return sre.ResponseMetadata.Messages
}
20 changes: 20 additions & 0 deletions pkg/channels/channelkinds/slack/blocks_degrade_test.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,20 @@
package slack

import (
"errors"
"testing"

slackapi "github.com/slack-go/slack"
"github.com/stretchr/testify/assert"
)

func TestBlocksRejectionDetail(t *testing.T) {
sre := slackapi.SlackErrorResponse{Err: "invalid_blocks"}
sre.ResponseMetadata.Messages = []string{"invalid block [json-pointer:/blocks/0/chart]: too many data points"}

// Error() surfaces only "invalid_blocks"; the actionable per-block reason
// lives in ResponseMetadata.Messages and would otherwise be dropped.
assert.Equal(t, sre.ResponseMetadata.Messages, blocksRejectionDetail(sre))
assert.Nil(t, blocksRejectionDetail(errors.New("network down")), "a non-Slack error carries no block detail")
assert.Nil(t, blocksRejectionDetail(nil))
}
78 changes: 38 additions & 40 deletions pkg/channels/channelkinds/slack/components.go
Original file line number Diff line number Diff line change
Expand Up @@ -55,8 +55,9 @@ const componentFormattingInstructions = `This channel can render structured Bloc
`"markdown" (a standard-markdown block, ≤12000 chars); ` +
`"table" (rows of rich_text cells; one table per message, ≤100 rows, ≤20 columns); ` +
`"data_visualization" (a chart; REQUIRES a "title" ≤50 chars and a "chart" object. ` +
`pie: {"type":"pie","segments":[{"label":"Organic","value":60}]}. ` +
`bar/area/line: {"type":"bar","series":[{"name":"Signups","data":[{"label":"Mon","value":10}]}],"axis_config":{"categories":["Mon","Tue"]}} — note each series uses "data" (not "data_points") and axis_config.categories is required; ≤2 charts per message); ` +
`pie: {"type":"pie","segments":[{"label":"Organic","value":60}]} — 1–6 segments. ` +
`bar/area/line: {"type":"bar","series":[{"name":"Signups","data":[{"label":"Mon","value":10}]}],"axis_config":{"categories":["Mon","Tue"]}} — each series uses "data" (not "data_points") and axis_config.categories is required. ` +
`Slack caps every chart: 1–6 series; 1–20 x-axis categories, all distinct; EXACTLY one data point per category per series, and each point's "label" must be the identical string as its category (not a reformatted date); bucket long time ranges into ≤20 points — do not emit a point per minute; all category labels, series names, and point labels 1–20 chars; pie segment values must be > 0 (omit zero buckets); ≤2 charts per message); ` +
`"plan" and "task_card" (task lists); ` +
`"card" (title/subtitle/body/subtext — NO action buttons, NO images yet); ` +
`"container" (groups 1–10 read-only child blocks in "child_blocks"; REQUIRES a "title" or "rich_text_title"). ` +
Expand Down Expand Up @@ -329,11 +330,14 @@ func validateTable(loc string, t *slackapi.TableBlock) error {
}

// validateDataViz enforces the data_visualization schema Slack's chat.postMessage
// requires (verified empirically against the live API): a non-empty title ≤50
// chars, a chart of a known type, and the per-type data shape — pie needs
// segments, bar/area/line need series plus axis_config.categories. Enforcing it
// here turns a would-be post-time invalid_blocks (which degrades the whole reply
// to plain text) into a precise error the agent can correct.
// requires by delegating to slack-go's DataVisualizationBlock.Validate, which
// models Slack's per-chart rules (verified against the live API): 1–6 series or
// pie segments, 1–20 unique categories, exactly one point per category whose
// label matches it, 1–20-char labels, pie values > 0, and the title/axis-label
// caps. Enforcing it here turns a would-be post-time invalid_blocks (which
// degrades the whole reply to plain text, sender.go) into a located error the
// agent can correct. The only rule kept locally is a whitespace-only title,
// which Validate's length check lets through.
//
// It also runs the no-mass-notification guard over EVERY free-form chart string
// — the title plus segment labels, series names, data-point labels, axis
Expand All @@ -345,60 +349,54 @@ func validateDataViz(loc string, dv *slackapi.DataVisualizationBlock) error {
if strings.TrimSpace(dv.Title) == "" {
return blockErrf(loc, "data_visualization", "requires a non-empty \"title\" (≤%d chars)", maxDataVizTitleLen)
}
if n := runeLen(dv.Title); n > maxDataVizTitleLen {
return blockErrf(loc, "data_visualization", "title is %d characters; Slack limits it to %d", n, maxDataVizTitleLen)
if err := dv.Validate(); err != nil {
return blockErrf(loc, "data_visualization", "%v. %s", err, dataVizLimitsHint)
}
if err := checkBroadcast(loc, "data_visualization", dv.Title); err != nil {
return err
}
var labels []string
switch c := dv.Chart.(type) {
case *slackapi.DataVisualizationPieChart:
if len(c.Segments) == 0 {
return blockErrf(loc, "data_visualization", "pie chart requires a non-empty \"segments\" array of {label,value}")
}
for _, seg := range c.Segments {
if err := checkBroadcast(loc, "data_visualization", seg.Label); err != nil {
return err
}
labels = append(labels, seg.Label)
}
case *slackapi.DataVisualizationBarChart:
return validateVizSeries(loc, "bar", c.Series, c.AxisConfig)
labels = vizSeriesStrings(c.Series, c.AxisConfig)
case *slackapi.DataVisualizationAreaChart:
return validateVizSeries(loc, "area", c.Series, c.AxisConfig)
labels = vizSeriesStrings(c.Series, c.AxisConfig)
case *slackapi.DataVisualizationLineChart:
return validateVizSeries(loc, "line", c.Series, c.AxisConfig)
labels = vizSeriesStrings(c.Series, c.AxisConfig)
default:
return blockErrf(loc, "data_visualization", "requires a \"chart\" object whose \"type\" is one of pie, bar, area, line")
}
for _, l := range labels {
if err := checkBroadcast(loc, "data_visualization", l); err != nil {
return err
}
}
return nil
}

func validateVizSeries(loc, kind string, series []slackapi.DataVisualizationDataSeries, axis slackapi.DataVisualizationAxisConfig) error {
if len(series) == 0 {
return blockErrf(loc, "data_visualization", "%s chart requires a non-empty \"series\" array; each series is {\"name\":…,\"data\":[{\"label\":…,\"value\":…}]}", kind)
}
if len(axis.Categories) == 0 {
return blockErrf(loc, "data_visualization", "%s chart requires \"axis_config\":{\"categories\":[…]} naming the x-axis categories", kind)
}
// dataVizLimitsHint is appended to a Validate failure so the model gets the
// whole rule set, not just the first rule its chart broke.
const dataVizLimitsHint = `Slack chart rules: 1–6 series (pie: 1–6 segments, each value > 0); ` +
`1–20 unique axis_config.categories (bucket long time ranges into ≤20 points); ` +
`each series has exactly one point per category, and each point's label must equal its category string; ` +
`every category, series name, and label is 1–20 characters`

// vizSeriesStrings collects the free-form strings of a bar/area/line chart for
// the broadcast guard.
func vizSeriesStrings(series []slackapi.DataVisualizationDataSeries, axis slackapi.DataVisualizationAxisConfig) []string {
out := make([]string, 0, len(axis.Categories)+2)
for _, s := range series {
if err := checkBroadcast(loc, "data_visualization", s.Name); err != nil {
return err
}
out = append(out, s.Name)
for _, p := range s.Data {
if err := checkBroadcast(loc, "data_visualization", p.Label); err != nil {
return err
}
out = append(out, p.Label)
}
}
for _, cat := range axis.Categories {
if err := checkBroadcast(loc, "data_visualization", cat); err != nil {
return err
}
}
if err := checkBroadcast(loc, "data_visualization", axis.XLabel); err != nil {
return err
}
return checkBroadcast(loc, "data_visualization", axis.YLabel)
out = append(out, axis.Categories...)
return append(out, axis.XLabel, axis.YLabel)
}

func validateContainer(loc string, c *slackapi.ContainerBlock) error {
Expand Down
67 changes: 66 additions & 1 deletion pkg/channels/channelkinds/slack/components_test.go
Original file line number Diff line number Diff line change
@@ -1,13 +1,54 @@
package slack

import (
"fmt"
"strings"
"testing"

"github.com/stretchr/testify/assert"
"github.com/stretchr/testify/require"
)

// vizLineChart builds a line-chart data_visualization block with `points`
// x-axis categories and one series carrying exactly one aligned point each —
// the shape a time-series answer produces. Used to exercise Slack's 1–20
// category/point cap.
func vizLineChart(points int) string {
cats := make([]string, points)
data := make([]string, points)
for i := range cats {
cats[i] = fmt.Sprintf(`"t%d"`, i)
data[i] = fmt.Sprintf(`{"label":"t%d","value":%d}`, i, i)
}
return `[{"type":"data_visualization","title":"p95","chart":{"type":"line",` +
`"series":[{"name":"p95","data":[` + strings.Join(data, ",") + `]}],` +
`"axis_config":{"categories":[` + strings.Join(cats, ",") + `]}}}]`
}

// vizManySeries builds a bar chart with `n` single-point series over one
// category, to exercise Slack's 1–6 series-per-chart cap.
func vizManySeries(n int) string {
series := make([]string, n)
for i := range series {
series[i] = fmt.Sprintf(`{"name":"s%d","data":[{"label":"Mon","value":%d}]}`, i, i)
}
return `[{"type":"data_visualization","title":"x","chart":{"type":"bar",` +
`"series":[` + strings.Join(series, ",") + `],` +
`"axis_config":{"categories":["Mon"]}}}]`
}

// vizBar builds a bar chart from a raw categories array and raw series
// objects, so each case states only the part of the chart it is breaking.
func vizBar(categories, series string) string {
return `[{"type":"data_visualization","title":"x","chart":{"type":"bar",` +
`"series":[` + series + `],"axis_config":{"categories":` + categories + `}}}]`
}

// vizPie builds a pie chart from raw segment objects.
func vizPie(segments string) string {
return `[{"type":"data_visualization","title":"x","chart":{"type":"pie","segments":[` + segments + `]}}]`
}

func repeatBlock(block string, n int) string {
parts := make([]string, n)
for i := range parts {
Expand Down Expand Up @@ -166,7 +207,7 @@ func TestValidateComponents(t *testing.T) {
{name: "rich_text usergroup element: rejected usergroup ping", json: `[{"type":"rich_text","elements":[{"type":"rich_text_section","elements":[{"type":"usergroup","usergroup_id":"S12345"}]}]}]`, wantErr: "usergroup"},
{name: "data_viz pie segment label <!channel>: rejected broadcast", json: `[{"type":"data_visualization","title":"Share","chart":{"type":"pie","segments":[{"label":"<!channel>","value":60}]}}]`, wantErr: "broadcast"},
{name: "data_viz bar series name <!subteam^>: rejected usergroup ping", json: `[{"type":"data_visualization","title":"Signups","chart":{"type":"bar","series":[{"name":"<!subteam^S1>","data":[{"label":"Mon","value":10}]}],"axis_config":{"categories":["Mon"]}}}]`, wantErr: "usergroup"},
{name: "data_viz axis category <!here>: rejected broadcast", json: `[{"type":"data_visualization","title":"Signups","chart":{"type":"line","series":[{"name":"S","data":[{"label":"Mon","value":10}]}],"axis_config":{"categories":["<!here>"]}}}]`, wantErr: "broadcast"},
{name: "data_viz axis category <!here>: rejected broadcast", json: `[{"type":"data_visualization","title":"Signups","chart":{"type":"line","series":[{"name":"S","data":[{"label":"<!here>","value":10}]}],"axis_config":{"categories":["<!here>"]}}}]`, wantErr: "broadcast"},

// --- one block slot is reserved for the sender's mention_only footer ---
{name: "49 blocks: accepted at the agent cap", json: repeatBlock(`{"type":"divider"}`, 49), wantErr: ""},
Expand All @@ -187,6 +228,30 @@ func TestValidateComponents(t *testing.T) {
{name: "table of 101 rows: rejected", json: `[` + rawTextTable(101, 1) + `]`, wantErr: "101 rows"},
{name: "table row of 21 cells: rejected", json: `[` + rawTextTable(1, 21) + `]`, wantErr: "21 cells"},

// --- data_visualization per-chart limits (Slack caps; slack-go
// block_data_visualization.go documents 1–6 series/segments, 1–20
// categories with exactly one point per category, ≤20-char labels) ---
{name: "line chart with 20 points: accepted at the cap", json: vizLineChart(20), wantErr: ""},
{name: "line chart with 21 points: rejected over cap", json: vizLineChart(21), wantErr: "cannot have more than 20 points"},
{name: "bar chart with 6 series: accepted at the cap", json: vizManySeries(6), wantErr: ""},
{name: "bar chart with 7 series: rejected over cap", json: vizManySeries(7), wantErr: "cannot have more than 6 series"},
{name: "series point count != categories: rejected", json: vizBar(`["Mon"]`, `{"name":"S","data":[{"label":"Mon","value":1},{"label":"Tue","value":2}]}`), wantErr: "exactly one point for every category"},
{name: "point label not matching its category: rejected", json: vizBar(`["2026-10-01","2026-10-02"]`, `{"name":"S","data":[{"label":"Oct 1","value":1},{"label":"Oct 2","value":2}]}`), wantErr: "must match axis_config.categories"},
{name: "repeated point label within a series: rejected", json: vizBar(`["Mon","Tue"]`, `{"name":"S","data":[{"label":"Mon","value":1},{"label":"Mon","value":2}]}`), wantErr: "duplicate label"},
{name: "repeated category: rejected", json: vizBar(`["Mon","Mon"]`, `{"name":"S","data":[{"label":"Mon","value":1},{"label":"Mon","value":2}]}`), wantErr: "duplicate category"},
{name: "empty category: rejected", json: vizBar(`[""]`, `{"name":"S","data":[{"label":"","value":1}]}`), wantErr: "categories[0] must have a minimum length of 1"},
{name: "category label over 20 chars: rejected", json: vizBar(`["`+strings.Repeat("c", 21)+`"]`, `{"name":"S","data":[{"label":"`+strings.Repeat("c", 21)+`","value":1}]}`), wantErr: "categories[0] cannot be longer than 20"},
{name: "empty series name: rejected", json: vizBar(`["Mon"]`, `{"name":"","data":[{"label":"Mon","value":1}]}`), wantErr: "name must have a minimum length of 1"},
{name: "series name over 20 chars: rejected", json: vizBar(`["Mon"]`, `{"name":"`+strings.Repeat("s", 21)+`","data":[{"label":"Mon","value":1}]}`), wantErr: "name cannot be longer than 20"},
{name: "empty data point label: rejected", json: vizBar(`["Mon"]`, `{"name":"S","data":[{"label":"","value":1}]}`), wantErr: "label must have a minimum length of 1"},
{name: "duplicate series names: rejected", json: vizBar(`["Mon"]`, `{"name":"S","data":[{"label":"Mon","value":1}]},{"name":"S","data":[{"label":"Mon","value":2}]}`), wantErr: "series names must be unique"},
{name: "pie chart with 7 segments: rejected over cap", json: `[{"type":"data_visualization","title":"x","chart":{"type":"pie","segments":[{"label":"a","value":1},{"label":"b","value":1},{"label":"c","value":1},{"label":"d","value":1},{"label":"e","value":1},{"label":"f","value":1},{"label":"g","value":1}]}}]`, wantErr: "more than 6 segments"},
{name: "pie segment label over 20 chars: rejected", json: vizPie(`{"label":"` + strings.Repeat("a", 21) + `","value":1}`), wantErr: "label cannot be longer than 20"},
{name: "pie segment with empty label: rejected", json: vizPie(`{"label":"","value":1}`), wantErr: "label must have a minimum length of 1"},
{name: "pie segment with zero value: rejected", json: vizPie(`{"label":"Sales","value":5},{"label":"Refunds","value":0}`), wantErr: "value must be greater than 0"},
{name: "pie segment with negative value: rejected", json: vizPie(`{"label":"Refunds","value":-1}`), wantErr: "value must be greater than 0"},
{name: "chart rejection: carries the full rule set for the model", json: vizLineChart(21), wantErr: "bucket long time ranges"},

// --- structurally empty blocks Slack rejects ---
{name: "header without text: rejected", json: `[{"type":"header"}]`, wantErr: "header"},
{name: "header with blank text: rejected", json: `[{"type":"header","text":{"type":"plain_text","text":" "}}]`, wantErr: "non-empty"},
Expand Down
4 changes: 2 additions & 2 deletions pkg/channels/channelkinds/slack/sender.go
Original file line number Diff line number Diff line change
Expand Up @@ -343,7 +343,7 @@ func (s *slackSender) Send(ctx context.Context, sess channelkinds.SessionInfo, e
// reply: retry the bubble update as plain text so the user still
// sees the answer — degraded, logged.
logger.Info("slack: streaming-bubble blocks rejected; retrying chat.update as plain text (degraded)",
"session", sess.Name, "channelID", channelID, "bubbleTS", ts, "err", err.Error())
"session", sess.Name, "channelID", channelID, "bubbleTS", ts, "err", err.Error(), "slackDetail", blocksRejectionDetail(err))
_, _, _, err = s.client.UpdateMessageContext(ctx, channelID, ts,
slackapi.MsgOptionText(responseReadyEmoji+" "+degradedPlainText(replyText, pl.Components, sess.Channel.RoutingMode), false))
}
Expand Down Expand Up @@ -420,7 +420,7 @@ func (s *slackSender) Send(ctx context.Context, sess channelkinds.SessionInfo, e
// (no Show-settings button), but delivered. Logged loudly so the
// degraded path is visible in operator logs.
logger.Info("slack: user_message blocks rejected; retrying as plain text (degraded, no Show-settings button)",
"session", sess.Name, "channelID", channelID, "err", err.Error())
"session", sess.Name, "channelID", channelID, "err", err.Error(), "slackDetail", blocksRejectionDetail(err))
usedBlocks = false
plainOpts := []slackapi.MsgOption{slackapi.MsgOptionText(degradedPlainText(replyText, pl.Components, sess.Channel.RoutingMode), false)}
if threadTS != "" {
Expand Down
2 changes: 1 addition & 1 deletion pkg/channels/channelkinds/slack/sender_plan.go
Original file line number Diff line number Diff line change
Expand Up @@ -249,7 +249,7 @@ func (s *slackSender) sendPlanUpdate(
newTS, wasPost, err := postOrUpdate(renderPlanBlocks(pl))
if err != nil && isUnsupportedBlocksErr(err) {
logger.Info("slack: plan/task_card blocks rejected; falling back to legacy emoji rendering",
"session", sess.Name, "err", err.Error())
"session", sess.Name, "err", err.Error(), "slackDetail", blocksRejectionDetail(err))
newTS, wasPost, err = postOrUpdate(renderPlanBlocksLegacy(pl))
}
if err != nil {
Expand Down
Loading