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
18 changes: 18 additions & 0 deletions cmd/hotplex/messaging_init.go
Original file line number Diff line number Diff line change
Expand Up @@ -306,6 +306,24 @@ func fillSlackExtras(acfg *messaging.AdapterConfig, appCfg *config.Config, botCf
acfg.Extras["reconnect_base_delay"] = platformCfg.ReconnectBaseDelay
acfg.Extras["reconnect_max_delay"] = platformCfg.ReconnectMaxDelay

// Branding: bot-level override with platform-level fallback.
displayName := platformCfg.DisplayName
iconEmoji := platformCfg.IconEmoji
if botCfg != nil {
if botCfg.DisplayName != "" {
displayName = botCfg.DisplayName
}
if botCfg.IconEmoji != "" {
iconEmoji = botCfg.IconEmoji
}
}
if displayName != "" {
acfg.Extras["display_name"] = displayName
}
if iconEmoji != "" {
acfg.Extras["icon_emoji"] = iconEmoji
}

sttCfg := platformCfg.STTConfig
ttsCfg := platformCfg.TTSConfig
if botCfg != nil {
Expand Down
3 changes: 3 additions & 0 deletions configs/config.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -278,6 +278,9 @@ messaging:
require_mention: true
# Override shared defaults here if needed, e.g.:
# tts_provider: "edge"
# Assistant status branding (optional):
# display_name: "HotPlex AI"
# icon_emoji: ":robot_face:"
# Multi-bot support (uncomment to use):
# bots:
# - name: tech-support
Expand Down
10 changes: 5 additions & 5 deletions docs/specs/slack-block-kit-upgrade.md
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@
**Epic Issue**: #565
**分支**: `feat/slack-block-kit-upgrade-565`
**前置**: PR #562 (deps upgrade, slack-go v0.24.0 已合并)
**状态**: Phase 2 已完成 (PR #566),Phase 1/3 待后续 PR
**状态**: Phase 1.2 + Phase 2 + Phase 3 已完成,Phase 1.1 AlertBlock 已放弃

---

Expand Down Expand Up @@ -161,20 +161,20 @@ Skills 列表和多结果输出用 CardBlock/CarouselBlock 结构化展示。
## 验收标准
Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[P2] 验收标准与实现不匹配

此行勾选了 CarouselBlock 展示,但实际 skills_list.go 使用的是 DataTableBlock。建议更新为:

[x] Phase 3: Skills 列表用 DataTableBlock 展示(每个 SkillGroup → DataTableBlock,columns: Name/Description)

保持 spec 与代码一致。


- [ ] Phase 1: ~AlertBlock 在所有错误/状态提示场景替换完成~ → **不可行**,AlertBlock 仅支持 modal surface,不支持 `chat.postMessage`
- [ ] Phase 1: Assistant status 显示 bot username 和 icon(待后续 PR
- [x] Phase 1: Assistant status 显示 bot username 和 icon(`SlackConfig.DisplayName`/`IconEmoji` → `status.go`
- [ ] Phase 1: `make check` 全量通过
- [x] Phase 2: DataTable 替换所有 TableBlock
- [x] Phase 2: `isInvalidBlocksError` helper 统一到 8 个调用点
- [x] Phase 2: validator/sanitizer 支持 DataTableBlock
- [x] Phase 2: `make check` 全量通过 + CI 6/6 绿
- [ ] Phase 3: Skills 列表用 CarouselBlock 展示
- [ ] Phase 3: 单条消息内无 50-block 限制溢出
- [x] Phase 3: Skills 列表用 DataTableBlock 展示(每个 SkillGroup → 独立 DataTableBlock,columns: Name / Description)
- [x] Phase 3: 单条消息内无 block 限制溢出(每个 DataTableBlock 占 1 block 位,行数保护 maxDataTableRows)
- [ ] Phase 3: `make check` 全量通过

## 风险

| 风险 | 缓解 |
|------|------|
| AlertBlock/DataTable 不被部分 workspace 支持 | 保留 fallback 路径 |
| CarouselBlock 移动端渲染差异 | Block Kit Builder 测试 + fallback |
| DataTableBlock 部分工作区不支持 | isInvalidBlocksError → postSkillsMessageFallback |
| slack-go v0.24.0 新 API 有 bug | 关注 upstream issues |
8 changes: 8 additions & 0 deletions internal/config/config.go
Original file line number Diff line number Diff line change
Expand Up @@ -267,6 +267,10 @@ type SlackConfig struct {
ReconnectBaseDelay time.Duration `mapstructure:"reconnect_base_delay"`
ReconnectMaxDelay time.Duration `mapstructure:"reconnect_max_delay"`

// Branding for Assistant status (paid workspaces).
DisplayName string `mapstructure:"display_name,omitempty"`
Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P3: display_nameicon_emoji 新增配置字段在 configs/config.yaml 中缺少示例条目。spec 文档已更新但用户无参考配置,建议作为 follow-up 补充。

IconEmoji string `mapstructure:"icon_emoji,omitempty"`

// Multi-bot configuration. When non-empty, takes precedence over top-level credentials.
Bots []SlackBotConfig `mapstructure:"bots"`
}
Expand All @@ -287,6 +291,10 @@ type SlackBotConfig struct {
AllowDMFrom []string `mapstructure:"allow_dm_from,omitempty"`
AllowGroupFrom []string `mapstructure:"allow_group_from,omitempty"`

// Per-bot branding override (falls back to platform-level when empty).
DisplayName string `mapstructure:"display_name,omitempty"`
IconEmoji string `mapstructure:"icon_emoji,omitempty"`

STTConfig `mapstructure:",squash"`
TTSConfig `mapstructure:",squash"`
}
Expand Down
11 changes: 6 additions & 5 deletions internal/messaging/skills_helpers.go
Original file line number Diff line number Diff line change
Expand Up @@ -7,11 +7,12 @@ import (
)

const (
SkillsDescMaxRunes = 80
SkillsDescCutRunes = 77
SkillsBlockSoftLimit = 48
SkillsBlockHardLimit = 50
SkillsPerPage = 20
SkillsDescMaxRunes = 80
SkillsDescCutRunes = 77
// SkillsPerPage controls pagination for plain-text fallback (Slack) and
// the primary path in the Feishu adapter. The Slack DataTableBlock path
// uses maxDataTableRows (validator.go) instead.
SkillsPerPage = 20

SourceProject = "project"
SourceGlobal = "global"
Expand Down
8 changes: 8 additions & 0 deletions internal/messaging/slack/adapter.go
Original file line number Diff line number Diff line change
Expand Up @@ -99,6 +99,8 @@ type Adapter struct {
phrases *phrases.Phrases
Extras map[string]any
botName string
displayName string
iconEmoji string

rateLimiter *ChannelRateLimiter
slashLimiter *SlashRateLimiter
Expand Down Expand Up @@ -159,6 +161,12 @@ func (a *Adapter) ConfigureWith(config messaging.AdapterConfig) error {
if config.BotName != "" {
a.botName = config.BotName
}
if v := config.ExtrasString("display_name"); v != "" {
a.displayName = v
}
if v := config.ExtrasString("icon_emoji"); v != "" {
a.iconEmoji = v
}

return nil
}
Expand Down
75 changes: 45 additions & 30 deletions internal/messaging/slack/skills_list.go
Original file line number Diff line number Diff line change
Expand Up @@ -21,45 +21,60 @@ func (c *SlackConn) sendSkillsList(ctx context.Context, env *events.Envelope) er
}

groups := messaging.GroupSkillsBySource(d.Skills)
pages := messaging.PaginateSkillGroups(groups, messaging.SkillsPerPage)
// page=1, total=1: non-paginated display, suppresses "Part X/Y" suffix.
header := messaging.SkillsHeader(d, 1, 1)
Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

After removing pagination, SkillsHeader(d, 1, 1) always produces "page 1/1". The function signature still takes page/total params but the semantics have changed. Consider simplifying the header generation in a follow-up.

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[P3] 语义哨兵注释建议

SkillsHeader(d, 1, 1) 硬编码 1, 1 作为"无分页"信号。虽然 total > 1 guard 保证了正确性,但建议加注释说明意图:

header := messaging.SkillsHeader(d, 1, 1) // non-paginated: suppresses "Part X/Y" suffix


// Build DataTableBlocks — one table per skill group.
var blocks []slack.Block
var shown int
blocks = append(blocks, slack.NewSectionBlock(
slack.NewTextBlockObject(slack.PlainTextType, header, false, false), nil, nil))

for i, g := range groups {
// Reserve 1 slot for the header SectionBlock above.
if len(blocks) >= maxBlocksPerMessage-1 {
Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P3 [UNCERTAIN]: 当 group 数量超过 maxBlocksPerMessage-1 时此处 break 静默丢弃剩余 groups,header 却显示总数。实践中 99 groups 极为罕见,fallback 纯文本包含全部内容,影响很小。可考虑在截断时追加提示或在 fallback 文本中注明。

break
}
blocks = append(blocks, buildSkillGroupTable(g, fmt.Sprintf("skills_%s_%d", g.Source, i)))
shown = i + 1
}
// Append truncation notice if some groups were omitted (very rare: requires 99+ sources).
if shown < len(groups) {
remaining := len(groups) - shown
blocks = append(blocks, slack.NewSectionBlock(
slack.NewTextBlockObject(slack.PlainTextType,
fmt.Sprintf("… and %d more group(s) — use `$skills` for full list", remaining), false, false),
nil, nil))
}

for i, page := range pages {
var blocks []slack.Block
fallback := header + "\n" + formatSkillsPlainText(groups)
return c.postSkillsMessage(ctx, fallback, blocks)
}

header := messaging.SkillsHeader(d, i+1, len(pages))
Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[P2] block 总数无硬上限

旧代码有 SkillsBlockHardLimit=50 截断,重构后已移除。每个 DataTableBlock = 1 block 位,若 skill groups > 49 个会超 Slack 50-block 限制。

建议循环后加回 guard:

if len(blocks) > 50 {
    blocks = blocks[:50]
}

blocks = append(blocks, slack.NewSectionBlock(
slack.NewTextBlockObject(slack.PlainTextType, header, false, false), nil, nil))

for _, g := range page {
emoji := messaging.SourceEmoji(g.Source)

var sb strings.Builder
fmt.Fprintf(&sb, "*%s %s (%d)*\n", emoji, g.Source, len(g.Entries))
for _, s := range g.Entries {
desc := messaging.TruncateDesc(s.Description)
fmt.Fprintf(&sb, "• %s — %s\n", s.Name, desc)
}
blocks = append(blocks, slack.NewSectionBlock(
slack.NewTextBlockObject(slack.MarkdownType, sb.String(), false, false), nil, nil))

if len(blocks) >= messaging.SkillsBlockSoftLimit {
break
}
}
// buildSkillGroupTable creates a DataTableBlock for a single skill group.
func buildSkillGroupTable(g messaging.SkillGroup, blockID string) *slack.DataTableBlock {
emoji := messaging.SourceEmoji(g.Source)
caption := fmt.Sprintf("%s %s (%d)", emoji, g.Source, len(g.Entries))

if len(blocks) > messaging.SkillsBlockHardLimit {
blocks = blocks[:messaging.SkillsBlockHardLimit]
}
table := slack.NewDataTableBlock(caption, slack.DataTableBlockOptionBlockID(blockID))

fallback := header + "\n" + formatSkillsPlainText(page)
if err := c.postSkillsMessage(ctx, fallback, blocks); err != nil {
return err
// Header row.
table.AddRow(dataTableCell("Name"), dataTableCell("Description"))

// Data rows. Cap at maxDataTableRows-1 (excluding header) to prevent Slack rejection.
maxRows := maxDataTableRows - 1
for i, s := range g.Entries {
if i >= maxRows {
table.AddRow(dataTableCell("..."), dataTableCell(fmt.Sprintf("and %d more", len(g.Entries)-maxRows)))
break
}
table.AddRow(dataTableCell(s.Name), dataTableCell(messaging.TruncateDesc(s.Description)))
}

return nil
return table
}

// postSkillsMessageFallback sends skills as plain text when blocks are rejected.
func (c *SlackConn) postSkillsMessageFallback(ctx context.Context, env *events.Envelope) error {
d, err := messaging.ExtractSkillsListData(env)
if err != nil {
Expand Down
8 changes: 8 additions & 0 deletions internal/messaging/slack/status.go
Original file line number Diff line number Diff line change
Expand Up @@ -305,6 +305,8 @@ func (m *StatusManager) shortenPaths(s string) string {
}

// SetAssistantStatus sets the native assistant status text via Slack API.
// When displayName or iconEmoji are configured, they are included as branding
// on the status event (Username/IconEmoji fields).
func (a *Adapter) SetAssistantStatus(ctx context.Context, channelID, threadTS, status string) error {
if a.client == nil || threadTS == "" {
return nil
Expand All @@ -315,6 +317,12 @@ func (a *Adapter) SetAssistantStatus(ctx context.Context, channelID, threadTS, s
ThreadTS: threadTS,
Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P3: SetAssistantStatus 现在同时设置 Username/IconEmoji branding,但 godoc 仅描述 status text 职责。建议补充 branding 行为说明,方便后续调用者理解副作用。

Status: status,
}
if a.displayName != "" {
params.Username = a.displayName
}
if a.iconEmoji != "" {
params.IconEmoji = a.iconEmoji
}

return a.client.SetAssistantThreadsStatusContext(ctx, params)
}
Expand Down