fix(slack): detect <@UID> mentions in mrkdwn blocks and legacy attachments

[chenwei791129]

What does this PR do?

#52387 fixed Block-Kit-only @mention detection for the one carrier Slack's WYSIWYG composer produces — a rich_text tree with a structured user element. Two other carriers were still dropped, so a bot that explicitly @-mentions the gateway stayed invisible to the allow_bots: mentions gate and to is_mentioned routing:

1. Hand-built blocks. An app that builds Block Kit by hand emits no user element — it writes the raw <@UID> token into a section/header/context block's text or fields string. _collect_slack_block_mentions recursed only through ("elements", "element") and appended only for type == "user" nodes, so neither the subtree nor the token was ever reached.
2. Legacy attachments. Detection returned early when event["blocks"] was falsy and never consulted attachments at all. #69316 had already established that Alertmanager, Grafana, PagerDuty and CI bots post with an empty top-level text and the real content inside attachments — but applied that understanding to display only, never to detection.

Both gaps close inside the existing helpers. The walker now also descends "text"/"fields" and harvests raw tokens from string values; a new _collect_slack_attachment_mentions covers the legacy carrier including attachment-nested blocks; a shared _SLACK_USER_MENTION_RE normalizes the labelled <@U123|alice> form to the bare token the gates compare against.

Why this approach. Two design decisions are worth calling out, because the obvious implementation of each is wrong:

• Recovered mentions are returned as a list, not spliced into the routing text. _slack_recovered_mentions hands the gates a token list; the routing text stays byte-identical to event["text"]. Appending recovered tokens would corrupt the two other consumers of that string. _slack_message_addressed_to_other_user reads its first token — and with an empty top-level text (precisely the alert-bot shape this fixes) the appended tail becomes that token, so the message would be dropped as "addressed to someone else". And user-configured wake-word patterns are matched with .search, so an anchored pattern like ^hey hermes$ would stop matching the moment a tail is appended. The gates now consume _slack_event_mentions_bot / _slack_mention_gate_inputs instead.
• The #52390 carve-out is generalized, not merely preserved. Widening what gets scanned adds carriers the structured rich_text_quote node check cannot see, and each is closed explicitly: mrkdwn blockquote markers, is_msg_unfurl/is_share attachments, and fallback. The underlying rule is not "quoting" but verbatim content — text the author is displaying rather than speaking — so code also counts: a <@UID> inside rich_text_preformatted, on a style.code element, or inside an mrkdwn triple-backtick fence or inline backtick span is not an address. Slack does not linkify mrkdwn inside code, so such a token notifies nobody; waking on it would be the same spurious trigger, reached through a different carrier. Without all of this, widening the scan would have re-opened the agent-agent re-trigger loop that allow_bots exists to prevent.

Related Issue

Fixes #75286

Type of Change

• 🐛 Bug fix (non-breaking change that fixes an issue)
• ✨ New feature (non-breaking change that adds functionality)
• 🔒 Security fix
• 📝 Documentation update
• ✅ Tests (adding or improving test coverage)
• ♻️ Refactor (no behavior change)
• 🎯 New skill (bundled or hub)

Changes Made

All production changes are in plugins/platforms/slack/adapter.py; all test changes in tests/gateway/test_slack_mention.py. 9 hunks in a 9232-line file — the diff is confined to the mention-detection helpers and their call sites, with no reformatting churn.

Detection helpers

• _SLACK_USER_MENTION_RE — one module-level pattern for <@UID> / <@UID|label>, capturing only the ID. The ID class is deliberately permissive: the gates substring-compare against whatever auth.test returned, so a narrower class would silently drop the very mention this recovers.
• _extract_mention_tokens() — the single place a mrkdwn string is scanned. Both carriers call it, so the carve-outs below apply to all of them automatically instead of having to be re-added per collector.
• _collect_slack_block_mentions() — _walk now descends "text" and "fields" too, harvesting tokens when the value is a string and the node is not verbatim. The flag (renamed from quoted, which no longer described what it carries) is set by rich_text_quote, rich_text_preformatted and style.code, and propagates down the subtree.
• _collect_slack_attachment_mentions() — new; covers pretext/title/text, fields[].title/value, and attachment-nested blocks via the existing block walker.
• _slack_recovered_mentions() — unions both sources and dedupes (one mention commonly appears in several carriers).
• _slack_mention_detection_text() — removed. It existed to hand a substring-testable string to the thread-parent wake check; that caller now takes a boolean (below), leaving the helper with no consumer. Keeping it would mean shipping a helper whose docstring warns against every remaining use of it.

Carve-outs preserving the #52390 contract

• Lines opening with a mrkdwn blockquote marker (>, and the escaped > form Slack actually sends) are skipped — mrkdwn-level quoting is invisible to the rich_text_quote node check.
• Code content is skipped in every carrier it has: the rich_text_preformatted node, style.code elements, and triple-backtick fences / inline backtick spans inside mrkdwn strings. This is the one carve-out that is genuinely new rather than extended — #52390 covered quoting only, so a mention a human formatted as code, or one an app emitted inside a payload dump or a relayed log line, still summoned the bot. That last shape lands squarely on the allow_bots: mentions path this PR serves. A human typing a literal token is unaffected either way, because Slack escapes it to <@U123>; that bound is now pinned by a test so relaxing _SLACK_USER_MENTION_RE cannot silently regress it.
• Attachments flagged is_msg_unfurl or is_share are skipped, mirroring the skip the agent-text path already performs at adapter.py:5483. Without this, pasting a permalink to an old <@BOT> deploy prod message would summon the bot — and, because the text path strips the unfurl, summon it with the body missing from its input.
• fallback is not scanned. Slack never renders it, so a mention living only there is invisible in the channel and notifies nobody.

Defensive handling

• The try/except is now per-attachment rather than around the whole loop. Previously one malformed sibling discarded the genuine mentions already collected from earlier attachments — reintroducing the exact silent drop this PR fixes.

Call sites

• allow_bots: mentions gate and is_mentioned routing now go through _slack_event_mentions_bot() / _slack_mention_gate_inputs().
• The #24848 thread-parent wake check is migrated too — its cached branch returned the raw msg["text"], empty for app-authored parents, so a plain follow-up reply in an alert thread was dropped after a restart. The fix otherwise would have covered a thread's first message but not its follow-ups. Rather than filtering display text, the two concerns are split:
  • _fetch_thread_parent_event() — new; returns the raw parent payload, cache-first.
  • _thread_parent_mentions_bot() — new; decides the wake through _slack_event_mentions_bot(), the same predicate the live channel gates use, so every carve-out above applies to the parent check by construction rather than by being re-added there.
  • _fetch_thread_parent_text() — back to display-only for reply_to_text injection, with a docstring saying it must not be substring-tested. Deriving the wake from it was unsafe in both directions: _render_message_text deliberately preserves rich_text_quote content for the agent to read, and extracts attachment text with no is_msg_unfurl/is_share exclusion and a fallback fallthrough. Its strip_bot_mention flag existed solely for the wake caller and is gone.

How to Test

1. Reproduce on main — both helpers are pure and module-level, so no gateway, credentials or config are needed. Run the snippet from #75286 against a plain checkout of main; all six shapes print DROPPED.

2. Verify the fix. On this branch the same snippet reports every carrier as seen.

3. Verify the carve-outs did not regress (these are the cases a naive fix breaks):

from plugins.platforms.slack.adapter import _slack_recovered_mentions as rec

BOT = "U0BOTID"
sect = lambda t: {"type": "section", "text": {"type": "mrkdwn", "text": t}}

# Pasted permalink / forwarded share must NOT wake the bot
print(rec({"text": "look at this",
           "attachments": [{"is_msg_unfurl": True, "text": f"<@{BOT}> deploy prod"}]}))  # []
# mrkdwn blockquote must NOT wake the bot
print(rec({"text": "", "blocks": [sect(f"&gt; <@{BOT}> old ping\nstatus: green")]}))     # []
# A malformed sibling must not discard a real mention
print(rec({"text": "", "attachments": [{"text": f"<@{BOT}> disk 91%"}, {"fields": 3}]}))  # ['<@U0BOTID>']

# Code content must NOT wake the bot, in any carrier
rt = lambda *e: {"type": "rich_text", "elements": list(e)}
print(rec({"text": "", "blocks": [rt({"type": "rich_text_preformatted",
           "elements": [{"type": "user", "user_id": BOT}]})]}))                          # []
print(rec({"text": "", "blocks": [sect(f"```\n notify <@{BOT}>\n```")]}))                # []
print(rec({"text": "", "blocks": [sect(f"the field holds `<@{BOT}>` verbatim")]}))        # []
# ...but the fence carve-out must end at the closing fence
print(rec({"text": "", "blocks": [sect(f"```\nlog\n```\n<@{BOT}> look")]}))              # ['<@U0BOTID>']

3b. Verify the thread-parent wake check obeys the same carve-outs. Both the cold and the cached parent path, since they are separate code paths that must agree:

# adapter = any SlackAdapter instance; parent = a thread-root payload
await adapter._thread_parent_mentions_bot(
    channel_id="C1", thread_ts=PARENT_TS, bot_uid=BOT)
# False for a parent whose mention lives in a quote / is_share / is_msg_unfurl /
# fallback / preformatted carrier; True for flat text, a section block, or an
# attachment field.

4. Verify the routing text stays clean — the two regressions the list-not-string design prevents:

adapter = ...  # any SlackAdapter instance
# Attachment-only alert naming a human must not look like "addressed to someone else"
routing_text, is_mentioned = adapter._slack_mention_gate_inputs(
    {"text": "", "attachments": [{"fields": [{"title": "owner", "value": "<@U_ONCALL>"}]}]}, BOT)
assert routing_text == "" and adapter._slack_message_addressed_to_other_user(routing_text, {BOT}) is False
# An anchored wake word must still match when an attachment is present

Both are covered by test_gate_leaves_routing_text_free_of_recovered_mentions and test_gate_wake_word_pattern_still_matches_with_an_attachment.

5. Run the suite:

pytest tests/gateway/test_slack_mention.py -q     # 87 passed (27 on main)
pytest tests/gateway/test_slack*.py -q            # 369 passed across 27 files

Of the 60 tests added to test_slack_mention.py, 53 fail on a plain main (verified by running the new file against the unmodified adapter); the other 7 are non-regression bounds that must pass both before and after — escaped tokens, text after a closed fence, a mention adjacent to an inline code span, an unpaired backtick, and rich_text_list carrying style as a plain string rather than a dict.

6. End-to-end. With allow_bots: mentions, have any app post a message with an empty top-level text and <@YOUR_BOT_UID> inside a section block or an attachment field. On main the gateway never replies; on this branch it does.

Checklist

Code

• I've read the Contributing Guide
• My commit messages follow Conventional Commits (fix(scope):, feat(scope):, etc.)
• I searched for existing PRs to make sure this isn't a duplicate
• My PR contains only changes related to this fix/feature (no unrelated commits) — single commit, two files, no ruff format churn
• I've run pytest tests/ -q and all tests pass — with one caveat, see note below
• I've added tests for my changes (required for bug fixes, strongly encouraged for features)
• I've tested on my platform: macOS 15.7.7 (Apple Silicon), Python 3.13.3

Note on pytest tests/ -q: tests/acp/test_entry.py fails to collect on a plain checkout (ModuleNotFoundError: No module named 'acp'), so I ran tests/gateway tests/plugins instead: 5550 passed, 3 failed. The 3 are pre-existing and unrelated — test_systemd_notify.py (1) and test_wecom_callback.py (2). I confirmed that by stashing this change and re-running those two files on the unmodified tree: the same 3 fail. Happy to rebase if they are fixed on main first.

Documentation & Housekeeping

• I've updated relevant documentation (README, docs/, docstrings) — docstrings on every touched helper; no user-facing docs affected
• N/A — no config keys added or changed
• N/A — no architecture or workflow change; the fix stays inside the existing Slack adapter helpers
• I've considered cross-platform impact (Windows, macOS) — pure string/dict handling, no platform-dependent code paths
• N/A — no tool descriptions or schemas changed

Screenshots / Logs

The failure is silent by design; the only trace on main is the existing debug line, which this PR also corrects (it claimed to cover "flat text or blocks" while attachments were never consulted):

# before
[Slack] Dropping bot message under allow_bots=mentions: no <@%s> mention in flat text or blocks

# after
[Slack] Dropping bot message under allow_bots=mentions: no <@%s> mention in flat text, blocks or attachments

[teknium1]

Thanks for the focused Slack fix. The premise is confirmed on current main: plugins/platforms/slack/adapter.py:341-344 only walks elements/element, and _slack_mention_detection_text() at :361-364 never reaches attachments.

Problems

• The thread-parent extension is not quote/share-safe on its cold-cache path. Proposed plugins/platforms/slack/adapter.py:7648 works from _render_message_text(), but that renderer intentionally preserves rich_text_quote content (:390-480) and extracts attachment text without an is_share exclusion (:483-521). The wake check then accepts any <@bot> substring (:5212-5222). This differs from the proposed cached path and can still let quoted/shared parent content wake a thread.

Suggested changes

• Derive the parent wake result from the raw parent event using the same filtered mention predicate used by the live gates, rather than the rendered display text. Add cold and cached parent tests for quoted, is_msg_unfurl, and is_share carriers.

Automated hermes-sweeper review.

[teknium1]

This cold-cache wake path still starts from _render_message_text(), which preserves rich_text_quote content and extracts attachments without an is_share exclusion. Because the caller wakes on a raw <@bot> substring, please derive the wake decision from the raw parent event via the same filtered mention predicate as the live gate, and add cold/cache tests for quote, unfurl, and share parents.

[chenwei791129]

Confirmed and fixed in 525a5199b.

Reproduced on 577e5be88 before changing anything, to pin which paths actually leak: the cold path woke on rich_text_quote, is_share, is_msg_unfurl and fallback-only parents, while the cached path did not — so the behavior also depended on whether the thread cache happened to be warm.

Rather than filtering the display text, the two concerns are now split:

• _fetch_thread_parent_event() — the raw parent payload, cache-first.
• _thread_parent_mentions_bot() — the wake decision, derived through _slack_event_mentions_bot(), the same predicate the live channel gates use. Every carve-out therefore applies to both parent paths by construction, instead of being re-added here where it can drift again.
• _fetch_thread_parent_text() is display-only again for reply_to_text injection, with a docstring saying it must not be substring-tested. Its strip_bot_mention flag existed solely for the wake caller and is gone.

_slack_mention_detection_text() had that same caller as its last consumer, so it is removed rather than left as a helper whose docstring warns against every remaining use of it; its tests now assert on the recovered-token list.

Tests as requested: cold × cached for quoted, is_msg_unfurl, is_share, fallback and preformatted parents, plus positive cases (flat text / section block / attachment field) so the #24848 wake itself stays pinned, plus one asserting _fetch_thread_parent_text still surfaces quoted content — that split is the point of the change.

Separately, 0f95ecfc4 closes the same class of leak one layer down, which the review prompted me to go looking for: a <@UID> inside rich_text_preformatted, on a style.code element, or inside an mrkdwn ``` fence / inline backtick span was still counted as a mention. Slack does not linkify mrkdwn inside code, so such a token notifies nobody — waking on it is the same spurious trigger the quote carve-out exists to prevent, so quoted and code content are now one verbatim class.

[GottZ]

This was generated by AI during triage.

Summary

Three PRs address this Slack mention-detection complex: #52390 and #52404 targeted structured Block Kit mentions for #52387, while #75312 extends detection to raw mrkdwn tokens, legacy attachments, and thread-parent wake paths for #75286.

Related pull requests

• fix(slack): detect Block-Kit-only @mentions in bot filter and router #52390 [closed] best fix — (+155/-2) — landed reference implementation: It introduced shared detection for structured Block Kit user elements across both mention gates, excluded rich_text_quote, added regression tests and observability, and remains relevant because it was cherry-picked with attribution and merged via fix(slack): extract Block Kit and attachment text in thread context and mention detection #69316.
• fix(slack): check Block Kit blocks for @mentions, not just flat text #52404 [closed] partial — (+8/-3) — superseded by fix(slack): extract Block Kit and attachment text in thread context and mention detection #69316: It covered only channel routing, left the earlier allow_bots: mentions filter unchanged, reused extraction that includes quoted content, and added no regression tests; this directly supports the contributor review and closure in favor of the quote-safe fix(slack): detect Block-Kit-only @mentions in bot filter and router #52390-based implementation.
• fix(slack): detect <@UID> mentions in mrkdwn blocks and legacy attachments #75312 best fix — (+820/-80) — keep open with a salvage path: Its diff extends the landed helpers to recover normalized <@UID> tokens from hand-built block strings and non-share legacy attachments, routes both gates through one predicate, and adds carrier and thread-parent tests. Consistent with the keep_open review on fix(slack): detect <@UID> mentions in mrkdwn blocks and legacy attachments #75312, the revised diff addresses the contributor's cold-cache objection by deriving cached and fetched parent wake decisions from the raw event through _slack_event_mentions_bot, with quote, unfurl, share, fallback, and genuine-mention coverage.

Duplicates

#52390 and #52404 were competing fixes for #52387; #52404 was superseded by the #52390-based implementation landed through #69316. #75312 is a follow-up for additional carriers rather than a duplicate of those fixes.

Suggested consolidation

Keep #75312 open with a salvage path focused on its distinct recoverable value: raw mrkdwn mention extraction, filtered legacy-attachment handling, a shared event-level predicate for both live and thread-parent gates, and the added regression matrix. Require validation of the revised head against the contributor's quote/share-safe cold-cache requirements before further disposition; #52390 and #52404 should remain closed as the landed reference and its superseded duplicate, respectively.

Complex graph

flowchart LR
    classDef open fill:#dbeafe,stroke:#1d4ed8,color:#1e3a8a
    classDef merged fill:#dcfce7,stroke:#15803d,color:#14532d
    classDef closed fill:#e5e7eb,stroke:#6b7280,color:#1f2937
    classDef unverified fill:#f3f4f6,stroke:#9ca3af,color:#374151
    classDef best stroke-width:3px,stroke:#b45309
    classDef target stroke-width:3px,stroke:#4338ca
    I75286(["issue #75286 (open)"])
    P75312["PR #75312 (open)"]
    P75312 -->|best fix| I75286
    class I75286 open
    class P75312 open
    class P75312 best
    class P75312 target
    click I75286 "https://github.com/NousResearch/hermes-agent/issues/75286"
    click P75312 "https://github.com/NousResearch/hermes-agent/pull/75312"

Loading

Graph: solid arrow = fixes / best fix, dashed arrow = partial or unverified (see edge label); boxed group = PRs duplicating each other; amber border = best fix; indigo border = target; gray node = closed (state tag in the node label).

Cross-PR triage: Reviewed 3 pull requests and 2 issues in this complex. Each diff was read against this issue; Assessment working set: 54 kB of PR diffs, 34 kB of issue/PR text, 8 kB of discussion (10 comments), 6 verify verdicts. verdicts reflect diff content, not PR titles. Part of an automated triage batch.

[chenwei791129]

rebase to latest main branch

[chenwei791129]

Updated this PR with the review fixes and production hardening for Slack attachment routing.

Slack attachment / unfurl handling

• Added a shared three-state classifier (content, share, unfurl) used by mention recovery, current-event rendering, and thread/parent rendering.
• content remains visible and may provide authored mention admission.
• share remains visible in current and historical context, but historical mentions and wake words inside it cannot grant routing control to the outer message. Share classification takes priority when Slack also supplies unfurl flags.
• unfurl is excluded from context and cannot contribute mentions or other routing control.
• Slack flags are accepted only when their value is literal True; malformed truthy values fail open as genuine content.
• Automatic URL previews are recognized only from URLs authored in the same message's top-level text or valid, non-verbatim Block Kit content. Quoted, forwarded, preformatted, code-styled, and malformed blocks cannot establish unfurl provenance.
• Classification fails open for uncertain legacy attachments, preserving Alertmanager/Grafana/PagerDuty/CI-style content.
• Added defensive URL normalization and malformed attachment handling.

Mention and workspace correctness

• Ignore literal mentions in plain_text, inline/fenced code, and blockquotes while preserving mentions after a same-line closed fence.
• Normalize and strip labelled mentions such as <@UID|name>.
• Scope remembered mentioned threads by workspace.
• Use the workspace-specific bot user ID for allow_bots=mentions and own-message echo filtering.

Attribution policy cleanup

• Removed the branch's automatic AI_AGENT / HERMES_AGENT exports and related docs/tests because repository policy requires a generic user-facing opt-in before enabling usage attribution.

Production regression covered

• An outer message such as test2 with an is_share: true attachment containing an old <@BOT> request no longer wakes the agent.
• If the outer message itself contains an authored bot mention, routing succeeds and the shared content remains available to the agent.
• Shared wake-word / authorization-like text is context only and does not admit the outer message.

Verification

• Complete Slack suite: 424 passed, 0 failed across 27 files.
• Targeted adapter suite: 176 passed.
• Mention suite: 122 passed.
• ruff check and git diff --check passed.

Commits: f2293bce1, 79eba8cfc, 8ab4f061c.