feat(slack): make stripping the bot's own mention configurable (slack.strip_bot_mentions) - #83905
Conversation
|
CI note: |
The adapter deletes the bot's own `<@U…>` token from the text before the agent reads it, and reports nothing in its place. A thread keeps waking the bot after the first mention, so every delivered turn then looks identical: the agent cannot tell "someone tagged me" from "I was woken by thread routing" — the distinction an agent needs to decide for itself whether a turn deserves an answer. Add `slack.strip_bot_mentions` (default `true` — today's behavior byte for byte, so nothing changes for anyone who does not opt in). With `false` the mention stays where the author put it, rendered as `@BotName`, the same shape `_humanize_user_mentions()` gives mentions of other participants; its absence then means the bot was woken by channel or thread routing. The asymmetry is the signal — no marker text is injected. The name comes from `_team_bot_names` / `_bot_display_name`, both resolved at connect time, so there is no extra Slack call; an unresolved name leaves the raw token rather than deleting it. Routing is untouched (`is_mentioned`, `_mentioned_threads`, `require_mention`, `strict_mention`, `thread_require_mention` are all evaluated before this), and so is command parsing, which runs off the separate `mention_stripped` variable. - `_slack_strip_bot_mentions()` resolves `config.extra` → `SLACK_STRIP_BOT_MENTIONS` → default `true`, like its four siblings. - Thread history follows the same policy (`_render_message_text`, `_format_thread_context`), so past turns don't read as "nobody ever tagged me"; block content is compared against the text as written, since the blocks carry the raw token. - `_fetch_thread_parent_text()` forwards the caller's `strip_bot_mention` into the render on a cache miss. That path is the root-mention wake check (NousResearch#24848), which greps the parent for the raw `<@id>`, and the render deleted it unconditionally — so on a cold cache the check could never match, in either flag state. - Documented in the Slack guide and the environment-variable reference; `config.yaml` is the canonical place, the env var is a mirror. - 51 tests over both flag states. Co-authored-by: Junie <junie@jetbrains.com>
6f3f12c to
ac62c19
Compare
feat(slack): make stripping the bot's own mention configurable (
|
|
Thanks for the pass. All five points are confirmations of intent rather than defects, so nothing changes here — but two of them deserve the reasoning on record rather than a bare "yes, intended". 1. Empty team name falls back to the primary display name. Deliberate. 2. 3. Raw 4 & 5. Yes — command routing was the thing I was most careful about ( |
What does this PR do?
Makes the deletion of the bot's own Slack mention optional, behind a new
slack.strip_bot_mentionsconfig field. Defaulttrueis today's behavior byte for byte — no existing test needed editing.The adapter deletes the bot's own
<@U…>token from the text before the agent reads it, and reports nothing in its place. That is fine while the adapter alone decides who gets an answer, but a thread keeps waking the bot after the first mention (_mentioned_threads, withthread_require_mentionoff by default), so every delivered turn then looks identical: the agent cannot tell "someone just tagged me" from "I was woken by thread routing".That is the distinction an operator needs when the agent — not the adapter — decides whether a turn deserves an answer at all (a "stay silent unless addressed" policy stated in the operator's own prompt). With
strip_bot_mentions: falsean explicit tag stays visible where the author put it, and its absence means the bot was woken by channel or thread routing. The asymmetry itself is the signal; no marker text is injected.The mention is rendered as
@BotName, the same shape_humanize_user_mentions()already gives mentions of other participants, so the agent reads an ordinary tag rather than a raw<@U…>id or a synthetic prefix. This is deliberately different from #47536, which was closed as superseded by mention humanization (#69483) because keeping raw<@U…>tokens would conflict with it — here the bot's own mention is humanized exactly like everyone else's, in place. The name comes from_team_bot_names/_bot_display_name, both resolved at connect time, so there is no extrausers.infocall per message; when the display name is not resolved yet the raw token is left alone rather than deleted.Routing is untouched:
is_mentioned,_mentioned_threads,require_mention,strict_mention,thread_require_mentionall keep their meaning and are all evaluated before this. Command parsing is untouched too — it runs off the separatemention_strippedvariable, so@bot /statusand@bot !newdispatch identically in both states.Related Issue
Related to #78106 (a Slack mention wakes the bot, but the mention is stripped before the agent's response decision). This gives the "keep
@hermesin the visible text" half of that issue's acceptance criteria as an opt-in field; it does not change the default, so it is not a full fix for a user who expects the new behavior out of the box.Type of Change
Changes Made
plugins/platforms/slack/adapter.py:_slack_strip_bot_mentions()sits with its four siblings (_slack_require_mention,_slack_strict_mention,_slack_ignore_other_user_mentions,_slack_thread_require_mention) and resolves the same way —config.extra→SLACK_STRIP_BOT_MENTIONS→ defaulttrue, with the explicit-false parsing_slack_require_mentionuses (the safe default here isTrue).plugins/platforms/slack/adapter.py:_own_bot_name()returns the connect-time display name for a workspace (the expression_build_identity_prompt()already used, now shared);_render_own_mention()rewrites<@U123>/<@U123|label>to@BotNamein place. There.subreplacement is a callable, since a display name is arbitrary user data and a backslash in it would otherwise raisere.errorand cost the message.plugins/platforms/slack/adapter.py: the strip site in_handle_slack_message()is now gated — strip, or render in place.plugins/platforms/slack/adapter.py: thread history follows the same policy, so past turns don't read as "nobody ever tagged me" —_render_message_text()takesstrip_bot_mention/bot_name, and_format_thread_context()passes the resolved flag through. When the mention is kept, block content is compared against the text as written: the blocks carry the raw token, so comparing them against a rendered@BotNamewould append the same authored message a second time.plugins/platforms/slack/adapter.py:_fetch_thread_parent_text()forwards the caller'sstrip_bot_mentioninto the render on a cache miss. That path is the root-mention wake check (fix(slack): route replies from mentioned thread parents #24848), which greps the parent for the raw<@id>, but_render_message_text()deleted the token unconditionally — so on a cold cache (i.e. the first reply after a restart, which is exactly when that check matters) it could never match. Reproduced with a stubbedconversations_replies, in both flag states, so this is a pre-existing defect the forwarding also fixes.hermes_cli/config_defaults.py:slack.strip_bot_mentions: True, documented in place.plugins/platforms/slack/plugin.yaml:optional_enventry for theSLACK_STRIP_BOT_MENTIONSmirror.website/docs/user-guide/messaging/slack.md: a subsection under the mention-gating options — what the key does, and explicitly what it does not change.website/docs/reference/environment-variables.md: the env mirror, pointing atconfig.yamlas canonical.tests/gateway/test_slack_strip_bot_mentions.py: 51 tests, most parameterized over both flag states.How to Test
@hermes what's up?still arrives aswhat's up?;tests/gateway/test_slack.py::test_channel_mention_strips_bot_idpins this and passes unedited.config.yaml:hey @hermes lookarrives ashey @hermes look— the tag in place, not a raw<@U…>and not moved to the front. A mention-only ping arrives as@hermes, not as an empty message.@hermes /statusand@hermes !newstill dispatch as/status//new, in both states.scripts/run_tests.sh tests/gateway/test_slack_strip_bot_mentions.py tests/gateway/test_slack.py tests/gateway/test_slack_mention.py tests/gateway/test_slack_mention_humanization.py tests/test_slack_thread_require_mention.py tests/gateway/test_config.py tests/gateway/test_slack_block_kit.py tests/gateway/test_slack_block_kit_adapter.py tests/gateway/test_slack_ignore_other_user_mentions.py tests/gateway/test_slack_require_mention_channels.py tests/gateway/test_slack_peer_agent_smoke.py— 348 passed.scripts/run_tests.sh tests/gateway/— 5293 passed, 7 failures reproduced on a clean checkout of this branch's base (macOS: optional XML dependency for wecom, Linux-only abstract sockets, api_server health/readiness/shutdown-forensics, multiplex busy-input).Checklist
Code
Documentation & Housekeeping
cli-config.yaml.example— N/A, the Slack defaults block lives inhermes_cli/config_defaults.py, which is updatedCONTRIBUTING.md/AGENTS.md— N/A, no architecture or workflow changeNotes
@bot 2,@bot always,@bot y) no longer exact-matches the clarify-choice, slash-confirm and pending-/updatebranches ingateway/run.py. That exposure is inherent to any approach that keeps the mention in the text, it only applies to operators who opt in, and guarding it would mean teaching the Slack adapter about gateway prompt state — happy to add a follow-up if you'd rather have it._render_message_text()arich_textblock mirroring a flat text that contains the bot token is still appended a second time, because the comparison is a plain substring check against the post-strip text. It is not a regression of this flag, so it is out of scope here.