feat(slack): opt-in Block Kit rendering for agent messages (salvage #56090) - #182
Conversation
|
Review Complete Files Reviewed: 6 By Severity:
Adds a markdown-to-Slack-Block-Kit renderer to the Slack adapter with 4 bugs found: rich_text_quote schema violation, broken inline nesting (links/code inside emphasis dropped), escaped-pipe GFM parsing bug, and unnecessarily capped indent levels. Files Reviewed (6 files) |
![tenki-reviewer[bot]](https://avatars.githubusercontent.com/in/2900641?s=60&v=4){kind=small}
There was a problem hiding this comment.
Risk: 🟠 High (72/100) — 1 high finding, 2 medium, 1 low · 865 LOC across 6 files
Summary
PR #182 introduces block_kit.py, a new markdown-to-Slack-Block-Kit compiler for the Slack messaging adapter, with corresponding tests and docs.
Findings
High — Broken inline nesting hierarchy (block_kit.py:120)
_walk_emphasis never re-enters _walk_links or walk (for inline code). Links and inline code inside bold/italic/strikethrough spans are silently dropped instead of being properly nested. Real agent output (e.g., **see [the docs](url)**) will lose its bold formatting on the link text.
Medium — rich_text_quote schema violation (block_kit.py:169)
_quote_block places raw element dicts directly into rich_text_quote.elements, bypassing the required rich_text_section wrapper. Slack may reject the entire blocks payload for this block, falling back to plain text.
Medium — Escaped-pipe GFM parsing bug (block_kit.py:244)
_split_row incorrectly absorbs \\| (backslash + pipe) as an escaped pipe. Per GFM, \| is a literal backslash followed by a real column separator, not an escaped pipe. Both native table and monospace fallback paths are affected.
Low — Indent cap too restrictive (block_kit.py:63)
_indent_level caps rich_text_list indent at 5, but Slack API supports indents 0–8. Deeply nested lists render at degraded indent levels.
Assessment
All bugs live in the new block_kit.py renderer — a self-contained plugin file. No core files are touched. The bugs cause degraded rendering (high and medium) but do not crash or leak data. Recommended: fix before merge.
| def _quote_block(lines: List[str]) -> Block: | ||
| section_children: List[Dict[str, Any]] = [] | ||
| for i, ln in enumerate(lines): | ||
| if i: | ||
| section_children.append({"type": "text", "text": "\n"}) | ||
| section_children.extend(_inline_elements(ln)) | ||
| return { | ||
| "type": "rich_text", | ||
| "elements": [{"type": "rich_text_quote", "elements": section_children}], | ||
| } |
There was a problem hiding this comment.
🟡 rich_text_quote block has invalid schema: raw text elements not wrapped in rich_text_section (bug)
The _quote_block function at block_kit.py:169-178 builds a rich_text_quote block whose elements array contains raw text/link elements (output of _inline_elements) and bare newline text separators, without wrapping each line in a rich_text_section object. Per Slack's Block Kit reference, rich_text_quote.elements must be an array of rich_text_section objects. The sibling _list_block at line 204-206 correctly wraps each item in rich_text_section, confirming this is an inconsistency/bug. Slack may reject or degrade the malformed block, causing the entire rich_blocks payload to fall back to the plain text field.
💡 Suggestion: Wrap each quoted line's inline elements in a rich_text_section. For multi-line quotes, emit separate rich_text_section elements — Slack handles line breaks between sibling rich_text_section children within a rich_text_quote natively. Remove the bare newline text element injection.
📋 Prompt for AI Agents
In plugins/platforms/slack/block_kit.py, rewrite _quote_block (lines 169-178). For each input line, create a rich_text_section element wrapping _inline_elements(ln), and put those sections directly into rich_text_quote.elements. Remove the newline text element injection. The corrected per-line structure: {"type": "rich_text_section", "elements": _inline_elements(ln)}. Multi-line quotes become a rich_text_quote element containing multiple rich_text_section children — Slack handles the line separation between sibling sections natively.
| # Temporarily protect escaped pipes, split on real ones, then restore. | ||
| protected = row.strip().strip("|").replace(r"\|", "\x00PIPE\x00") | ||
| return [c.strip().replace("\x00PIPE\x00", "|") for c in protected.split("|")] |
There was a problem hiding this comment.
🟡 Escaped-pipe handling in _split_row incorrectly absorbs '\|' (GFM: literal backslash + real pipe) (bug)
In block_kit.py:238-246, _split_row uses row.strip().strip('|').replace(r'\|', '\x00PIPE\x00') to protect escaped pipes before splitting. This naive string replacement treats \| (GFM: literal backslash followed by a real pipe column separator) as an escaped pipe because the trailing | matches. The result: a cell containing a literal backslash absorbs the following column separator. Correct GFM handling requires a left-to-right scan tracking whether each backslash is itself escaped (odd consecutive backslashes before a pipe = escaped; even = real separator). Both the native table block path and the monospace preformatted fallback are affected since both call _split_row.
💡 Suggestion: Replace the naive str.replace with a character-by-character scan that tracks consecutive backslash counts. Only treat a pipe preceded by an odd number of backslashes as escaped. This correctly handles \| (even count -> backslash is escaped, pipe is a real separator) vs. \| (odd count -> pipe is escaped).
📋 Prompt for AI Agents
In plugins/platforms/slack/block_kit.py, _split_row function (lines 238-246). Replace the row.strip().strip('|').replace(r'\|', '\x00PIPE\x00') approach with a left-to-right scan that counts consecutive backslashes before each pipe character. A pipe preceded by an odd number of unescaped backslashes is an escaped pipe (emit as literal pipe in the cell); a pipe preceded by an even number of backslashes (including 0) is a real column separator. Accumulate cell content and only split on real pipes.
| def _walk_emphasis(s: str, style: Dict[str, bool]) -> None: | ||
| if not s: | ||
| return | ||
| # Try bold, then strike, then italic, recursing into the inner span. | ||
| for rx, key in ((_BOLD_RE, "bold"), (_STRIKE_RE, "strike"), (_ITALIC_RE, "italic")): | ||
| m = rx.search(s) | ||
| if m: | ||
| _walk_emphasis(s[:m.start()], style) | ||
| inner_style = dict(style) | ||
| inner_style[key] = True | ||
| _walk_emphasis(m.group(1), inner_style) | ||
| _walk_emphasis(s[m.end():], style) | ||
| return | ||
| emit_text(s, dict(style) if style else None) |
There was a problem hiding this comment.
🟠 Links and inline code inside emphasis spans are silently dropped — nesting hierarchy broken in inline parser (bug)
The _walk_emphasis function at block_kit.py:120-133 processes emphasis-inner content (bold/italic/strikethrough captures) by recursively calling only _walk_emphasis. It never re-enters _walk_links (for link detection) or walk (for inline code detection). The outer-inner nesting chain is walk -> _walk_links -> _walk_emphasis, but once inside emphasis all higher-priority inline elements (links and code) are invisible. Input like link renders as literal ** markers around a correctly-styled link element (bold is lost). Input like code renders the backticks as literal bold text rather than bold code-styled text. Real agent output frequently contains links inside emphasized text.
💡 Suggestion: In _walk_emphasis, replace recursive _walk_emphasis() calls with walk() calls so that inline code and links within emphasis spans are detected. The walk function is in scope as a closure within _inline_elements. The change is 3 tokens: _walk_emphasis -> walk on lines 127, 130, and 131.
| def _walk_emphasis(s: str, style: Dict[str, bool]) -> None: | |
| if not s: | |
| return | |
| # Try bold, then strike, then italic, recursing into the inner span. | |
| for rx, key in ((_BOLD_RE, "bold"), (_STRIKE_RE, "strike"), (_ITALIC_RE, "italic")): | |
| m = rx.search(s) | |
| if m: | |
| _walk_emphasis(s[:m.start()], style) | |
| inner_style = dict(style) | |
| inner_style[key] = True | |
| _walk_emphasis(m.group(1), inner_style) | |
| _walk_emphasis(s[m.end():], style) | |
| return | |
| emit_text(s, dict(style) if style else None) | |
| def _walk_emphasis(s: str, style: Dict[str, bool]) -> None: | |
| if not s: | |
| return | |
| # Try bold, then strike, then italic, recursing into the inner span. | |
| for rx, key in ((_BOLD_RE, "bold"), (_STRIKE_RE, "strike"), (_ITALIC_RE, "italic")): | |
| m = rx.search(s) | |
| if m: | |
| walk(s[:m.start()], style) | |
| inner_style = dict(style) | |
| inner_style[key] = True | |
| walk(m.group(1), inner_style) | |
| walk(s[m.end():], style) | |
| return | |
| emit_text(s, dict(style) if style else None) |
📋 Prompt for AI Agents
In plugins/platforms/slack/block_kit.py, in the _walk_emphasis function (lines 120-133), change the three recursive calls from _walk_emphasis(...) to walk(...): line 127 prefix call, line 130 inner-content call, and line 131 suffix call. This re-enters the full inline pipeline (code -> links -> emphasis) within emphasis spans so that inline code and links inside bold/italic/strikethrough text are correctly detected and styled.
| width = 0 | ||
| for ch in spaces: | ||
| width += 4 if ch == "\t" else 1 | ||
| return min(width // 2, 5) # Slack rich_text_list supports up to indent 5 |
There was a problem hiding this comment.
🟢 rich_text_list indent capped at 5 but Slack API supports indents 0-8 (bug)
At block_kit.py:63, _indent_level returns min(width // 2, 5), capping rich_text_list indent at 5. The accompanying comment states 'Slack rich_text_list supports up to indent 5' — but the Slack API reference documents indent as an integer 0-8. Markdown lists nested 6+ levels deep all render at indent 5 on Slack instead of showing their true nesting depth. Visual hierarchy is degraded but no content is lost.
💡 Suggestion: Change the cap from 5 to 8 and update the comment.
| return min(width // 2, 5) # Slack rich_text_list supports up to indent 5 | |
| return min(width // 2, 8) # Slack rich_text_list supports indents 0-8 |
📋 Prompt for AI Agents
In plugins/platforms/slack/block_kit.py, line 63, change min(width // 2, 5) to min(width // 2, 8) and update the trailing comment from '# Slack rich_text_list supports up to indent 5' to '# Slack rich_text_list supports indents 0-8'.
1 participant
Infographic
Summary
Slack agent replies can now render as structured Block Kit — headers, dividers, true nested lists, blockquotes, code, and native
tableblocks — behind an opt-in flag (platforms.slack.extra.rich_blocks: true, default off). Salvage of NousResearch#56090 by @benbarclay onto currentmain. Closes NousResearch#18918.Changes
plugins/platforms/slack/block_kit.py(new): purerender_blocks(markdown)— headers, dividers,rich_textnested lists, blockquotes, preformatted code, and nativetableblocks with per-column alignment and inline-formattedrich_textcells. Enforces Slack's 50-block / 3000-char / table (100 rows · 20 cols · 10k chars) limits; over-limit or unparseable tables fall back to aligned monospace. Never raises →Noneon any unexpected input.plugins/platforms/slack/adapter.py:send()renders blocks on the single-chunk primary message;edit_message()renders blocks only onfinalize=True(streaming edits stay plain mrkdwn). Atext=fallback is always sent alongside blocks.website/docs/user-guide/messaging/slack.md+ zh-Hans i18n — config example + key-table row._rich_blocks_enableddocstrings, which still described the earlier monospace-only table approach.Validation
tests/gateway/test_slack_block_kit.py+_adapter.py— 27/27 passtableblock, alignment (left→null, right emitted), in-cell bold + link survivePlugin-only PR (touches
plugins/platforms/slack/,tests/gateway/,website/docs/), contributor authorship preserved via rebase-merge.Mirror-of: NousResearch#56102
NousResearch#56102