fix(slack): check Block Kit blocks for @mentions, not just flat text - #52404
fix(slack): check Block Kit blocks for @mentions, not just flat text#52404AlexFucuson9 wants to merge 1 commit into
Conversation
Mention detection only checked the flat field for @mentions. Block Kit messages that have the @mention only in (e.g. rich_text sections with user elements) were silently dropped. Extract text from blocks and include it in the mention check so @mentions in Block Kit blocks are properly detected. Fixes NousResearch#52387
tonydwb
left a comment
There was a problem hiding this comment.
Code Review Summary
Verdict: Approved
Small, focused fix for Slack Block Kit mention detection. The change correctly extracts text from blocks and includes it in the mention check. Note: PR #52390 addresses the same issue (#52387) with a more comprehensive approach including helper functions and additional test coverage — the author/reviewer should reconcile these.
Looks Good
- Minimal diff (8 additions, 3 deletions)
- Correct approach: merge blocks text into mention detection
- References the existing
_extract_text_from_slack_blockshelper
Reviewed by Hermes Agent
teknium1
left a comment
There was a problem hiding this comment.
Thank you for the focused Slack investigation. The current main code still has the reported Block Kit mention gap, but this patch needs rework before it can safely address it.
Problems
- The patch changes only channel routing. The earlier
allow_bots: mentionsfilter still reads flatevent["text"]and returns atplugins/platforms/slack/adapter.py:2607-2610, so the issue's bot-message reproduction remains dropped. _extract_text_from_slack_blocks()recursively includesrich_text_quotecontent (plugins/platforms/slack/adapter.py:173-174) and rendersuserelements as mention tokens (:145-146). Feeding all extracted text into routing would violate the existing quoted-mention safety contract intests/gateway/test_slack.py:1687-1721.- The PR adds no regression tests for either mention gate or for preserving quoted-content isolation.
Suggested changes
- Share a detection helper between the two gates, collecting only non-quoted Block Kit user mentions.
- Add integration tests for a direct Block Kit mention,
allow_bots: mentions, and quoted-only mention rejection.
Automated hermes-sweeper review.
| # text field (#52387). | ||
| blocks = event.get("blocks") | ||
| blocks_text = _extract_text_from_slack_blocks(blocks) if blocks else "" | ||
| mention_text = (original_text or "") + ("\n" + blocks_text if blocks_text else "") |
There was a problem hiding this comment.
_extract_text_from_slack_blocks() includes nested rich_text_quote content, so this makes a quoted/forwarded <@bot> satisfy routing. Current main deliberately rejects that case in tests/gateway/test_slack.py:1687-1721; use a detector that collects only non-quoted user elements instead.
|
Closing as superseded by #69316 (merged): counting mentions in ALL block text (incl. quoted/forwarded) risks response hijack; the landed fix (#52390 base) only counts non-quoted mentions. Thanks for digging into this — the consolidated fix stands on the cluster's collective analysis, and your work is credited in #69316's summary. |
Problem
Mention detection only checked the flat
textfield for @mentions. Block Kit messages that have the @mention only inblocks(e.g.rich_textsections withuserelements) were silently dropped.Files changed
plugins/platforms/slack/adapter.pyFixes #52387