Fix Slack notifications: 'fields' is not a Block Kit block type#474
Fix Slack notifications: 'fields' is not a Block Kit block type#474richard-devbot wants to merge 1 commit into
Conversation
Slack notifications have never been deliverable. Both payload builders in
src/notifications/index.js emitted a block of `{ type: 'fields', fields }`,
but Block Kit has no `fields` block type — fields belong on a `section`
block. Slack rejects the entire attachment with:
400 invalid_attachments
Verified against a live Incoming Webhook: failing before this change,
delivering successfully after it.
The bogus type had propagated to every consumer, so each one is updated to
read fields off a section block while still tolerating the old shape, which
keeps any in-flight or persisted payloads converting correctly:
- channels/teams.js - facts extraction
- channels/discord.js - embed fields
- channels/text.js - plain-text flattening (covered by the existing
"slackPayloadToText renders blocks as readable
plain text" test, which caught this consumer)
Teams and Discord converters produce identical output to before the change.
tests/notifications-channels.test.js, harness-notifications.test.js and
notify-hook.test.js pass 21/21.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Strix Security ReviewNo security issues found. Updated for Reviewed by Strix |
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Repository UI Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (4)
📝 WalkthroughWalkthroughSlack notification formatters now emit valid section blocks containing fields, while Discord, Teams, and text conversions accept fields from both section and legacy fields blocks. ChangesSlack fields compatibility
Estimated code review effort: 2 (Simple) | ~10 minutes Suggested reviewers: Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
PR Summary by QodoFix Slack Block Kit payloads: move
AI Description
Diagram
High-Level Assessment
Files changed (4)
|
Code Review by Qodo
Context used✅ Compliance rules (platform):
300 rules✅ Skills:
|
| // Fields ride on a section block; 'fields' is tolerated for older payloads. | ||
| if ((block.type === 'section' || block.type === 'fields') && block.fields) { | ||
| for (const f of block.fields) { |
There was a problem hiding this comment.
1. Legacy fields branch untested 📜 Skill insight ▣ Testability
A new backward-compatibility branch was added so Slack block parsing accepts both section blocks with fields and legacy block.type === 'fields', but the existing notification channel tests only exercise the section path. This leaves the legacy conversion path unverified and risks regressions for persisted/in-flight payloads shipping unnoticed.
Agent Prompt
## Issue description
Converters now accept both `section` blocks with `fields` and legacy `block.type === 'fields'`, but current tests only exercise the new `section` shape via `formatSlackStageMessage`, leaving the backward-compatibility branch untested.
## Issue Context
This PR explicitly aims to tolerate older persisted/in-flight Slack payloads; without a dedicated test that includes a legacy `{ type: 'fields', fields: [...] }` block, that compatibility guarantee can regress silently. Add a test that feeds a legacy Slack payload using `type: 'fields'` into each converter (e.g., `slackPayloadToText`, `convertSlackToDiscord`, `convertSlackToTeams`) and asserts the expected extracted fields / outputs.
## Fix Focus Areas
- tests/notifications-channels.test.js[19-42]
- src/notifications/channels/discord.js[20-33]
- src/notifications/channels/teams.js[17-40]
- src/notifications/channels/text.js[21-34]
ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools
Problem
Slack notifications have never been deliverable. Both payload builders in
src/notifications/index.js(formatSlackStageMessage,formatSlackTaskReportMessage) emit a block of:Block Kit has no
fieldsblock type — fields belong on asectionblock. Slack rejects the entire attachment:Because the failure is at the attachment level, no part of the message is delivered. Notifications are also fire-and-forget by design (
router.js), so this failed silently in normal operation.Reproduction
Against a live Incoming Webhook, with
RSTACK_SLACK_WEBHOOKset:After this change, same webhook:
Message confirmed delivered to the channel.
Fix
Producer now emits
type: 'section'withfields, which is the correct Block Kit shape.The bogus type had propagated to every consumer, so each is updated to read fields off a section block while still tolerating the old
'fields'shape — that keeps any persisted or in-flight payloads converting correctly:src/notifications/index.jschannels/teams.jschannels/discord.jschannels/text.jsVerification
tests/notifications-channels.test.js,tests/harness-notifications.test.js,tests/notify-hook.test.js— 21/21 pass.slackPayloadToText renders blocks as readable plain texttest caughttext.js, a consumer missed on the first pass — worth noting the suite did its job here.Note for maintainers
Separately from this fix: nothing loads a project's
.envintoprocess.env.resolveChannelsreadsprocess.envand.rstack/notifications.jsononly, whilecore/harness/env-file.jsis used solely by the Hub UI to edit keys. Following thedocs/integrations/webhooks.mdguidance to putRSTACK_SLACK_WEBHOOKin.envtherefore does not enable the channel unless the variable is also exported into the real environment. Left out of scope here — happy to open a separate issue.🤖 Generated with Claude Code
Summary by CodeRabbit