Skip to content

fix(slack): wire up mention_patterns config - #35403

Closed
kuk1song wants to merge 1 commit into
NousResearch:mainfrom
kuk1song:fix/slack-mention-patterns
Closed

fix(slack): wire up mention_patterns config#35403
kuk1song wants to merge 1 commit into
NousResearch:mainfrom
kuk1song:fix/slack-mention-patterns

Conversation

@kuk1song

@kuk1song kuk1song commented May 30, 2026

Copy link
Copy Markdown

What does this PR do?

telegram.py (PR #3870) and dingtalk.py (PR #11564) both implement mention_patterns, which lets a platform adapter treat a configured regex match as a wake trigger (for example, wake on a leading bot name or on any URL) even when the message does not @mention the bot.

PR #4644 (merged 2026-04-02) documented slack.mention_patterns in website/docs/user-guide/messaging/slack.md. The Slack adapter, however, never implemented it. The YAML key is silently accepted by gateway/config.py's generic platform-key loop and then ignored, and gateway/platforms/slack.py has zero references to mention_patterns on main. Anyone who follows the Slack docs and sets slack.mention_patterns gets a silent no-op.

This PR closes that docs-vs-code drift by giving Slack the same mention_patterns support Telegram and DingTalk already ship.

Surfaced while wiring a Slack summarizer bot on Hermes: the documented slack.mention_patterns had no effect, because the adapter never reads the key.

Example config.yaml

slack:
  mention_patterns:
    - "^\\s*hermes\\b"     # wake on a leading "hermes"
    - "https?://"          # wake on any URL

Behavior

When a channel message does not @mention the bot, it is now accepted as a trigger if it matches any configured regex, alongside the existing reply-to-bot, mentioned-thread, and active-session conditions.

strict_mention: true still requires an explicit @mention every turn; regex triggers cannot bypass it (matches Telegram's semantics). DMs are unaffected. When mention_patterns is absent or empty, the channel gate chain is byte-identical to current behavior.

Related Issue

No dedicated tracking issue; the gap surfaced from PR #4644 documenting slack.mention_patterns without the matching adapter implementation landing.

A broader prior attempt (#20723 / #20726) bundled this same mention_patterns wiring with unrelated Slack changes (strict-mode behavior tweaks, slash-command aliases, run_agent adjustments) across 6 files. It was withdrawn by its author and not reviewed on merits. This PR isolates only the documented mention_patterns support plus tests (3 files, +189 / -2), mirroring the existing telegram.py / dingtalk.py implementations.

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

  • gateway/platforms/slack.py: add _compile_mention_patterns() and _message_matches_mention_patterns(text) helpers; __init__ caches the compiled patterns. The channel gate chain accepts a regex match as a valid trigger alongside the thread and session conditions. Mirrors telegram.py:4568, 4753, 4824 and dingtalk.py:419, 487, 518.
  • gateway/config.py: bridge slack.mention_patterns from YAML to SLACK_MENTION_PATTERNS, keeping Slack symmetric with the existing per-platform bridges (TELEGRAM_MENTION_PATTERNS, WHATSAPP_MENTION_PATTERNS, DINGTALK_MENTION_PATTERNS).
  • tests/gateway/test_slack_mention.py: 12 new cases covering empty default, regex matching, case-insensitivity, single-string form, invalid-regex skipping, non-list rejection, empty-text early return, env-var JSON / multiline fallback, config-extra precedence over env, YAML-to-env bridging, and env-already-set precedence.

How to Test

  1. Add slack.mention_patterns to ~/.hermes/config.yaml:
    slack:
      mention_patterns:
        - "^\\s*hermes\\b"
        - "https?://"
  2. Start the Slack gateway and send channel messages without @mentioning the bot:
    • "hello there" → silently ignored (no pattern match)
    • "hermes what's up" → bot responds (matches ^\s*hermes\b)
    • "check https://example.com" → bot responds (matches https?://)
  3. Set slack.strict_mention: true and repeat. All three messages should be ignored unless the bot is explicitly @mentioned (regex triggers do not bypass strict mode, by design).
  4. Remove mention_patterns from config. Behavior is byte-identical to current main.

Automated coverage on this branch (rebased onto 5f84c9144):

  • bash scripts/run_tests.sh tests/gateway/ passes 273 files / 6032 tests / 0 failed in 44.0s.
  • bash scripts/run_tests.sh tests/gateway/test_slack_mention.py passes 67/67 (55 pre-existing + 12 new).
  • python scripts/check-windows-footguns.py clean on the three changed files.
  • ruff check clean (PLW1514).

Checklist

Code

Documentation & Housekeeping

  • Documentation already exists from PR docs: add Configuration Options section to Slack docs #4644 (website/docs/user-guide/messaging/slack.md, the mention_patterns: block); no further docs changes needed.
  • N/A. cli-config.yaml.example has no slack: block today (and no per-platform mention_patterns examples for Telegram / WhatsApp / DingTalk either), so there is nothing to extend for parity.
  • N/A. No architecture or workflow changes.
  • I've considered cross-platform impact: gateway code is platform-neutral, no OS-specific calls added, check-windows-footguns.py passes.
  • N/A. No tool schema changes.

Screenshots / Logs

Expected channel-message behavior with the example config (covered by tests/gateway/test_slack_mention.py, 67/67 passing):

"hello there"             → ignored        (no pattern matches)
"hermes what's up"        → bot responds   (matches ^\s*hermes\b)
"check https://x.com"     → bot responds   (matches https?://)
strict_mention: true      → all ignored unless explicitly @mentioned

Adjacent in-flight work

PR #29393 (open, currently conflicting) adds the same feature on the Discord side and introduces shared helpers in gateway/platforms/base.py. If it merges first, I will rebase onto those helpers.

@alt-glitch alt-glitch added type/feature New feature or request P2 Medium — degraded but workaround exists platform/slack Slack app adapter comp/gateway Gateway runner, session dispatch, delivery labels May 30, 2026
@kuk1song
kuk1song marked this pull request as ready for review May 30, 2026 23:39
Implements `slack.mention_patterns` (regex wake-word triggers in
channels), documented in website/docs/user-guide/messaging/slack.md
since PR NousResearch#4644 but never wired up. The YAML key was silently accepted
by gateway/config.py's generic platform-key loop and then ignored —
gateway/platforms/slack.py had zero references to mention_patterns.

Mirrors the existing implementations in telegram.py and dingtalk.py:

- gateway/platforms/slack.py
  - _compile_mention_patterns(): reads config.extra["mention_patterns"]
    and falls back to SLACK_MENTION_PATTERNS env var (JSON / newline /
    comma-separated, in that order). Invalid regex patterns are skipped
    with a warning; remaining patterns still compile. Non-list /
    non-string values log a warning and resolve to [].
  - _message_matches_mention_patterns(text): True iff any compiled
    pattern matches.
  - __init__: cache compiled patterns on the adapter.
  - Channel gate chain: a regex hit is now a valid trigger alongside
    reply_to_bot_thread / in_mentioned_thread / has_session. The
    strict_mention=true escape hatch still bypasses regex triggers,
    matching Telegram's semantics.

- gateway/config.py: bridge slack.mention_patterns from YAML to the
  SLACK_MENTION_PATTERNS env var, mirroring the existing Telegram /
  WhatsApp / DingTalk bridges. The generic bridged["mention_patterns"]
  path already routed the YAML value to config.extra; this extra line
  keeps Slack symmetric with the other three platforms for deployments
  that prefer env-var configuration.

- tests/gateway/test_slack_mention.py: 12 new cases covering empty
  default, regex matching, case-insensitivity, single-string form,
  invalid-regex skipping, non-list rejection, empty-text early return,
  env-var JSON / multiline fallback, config-extra precedence over env,
  YAML→env bridging, and env-already-set precedence.

Backward compatible: when mention_patterns is absent or empty the
channel gate chain is byte-identical to current behavior. The full
gateway test suite passes (273 files, 6032 tests, 0 failed).
@kuk1song
kuk1song force-pushed the fix/slack-mention-patterns branch from 482b855 to 93a7759 Compare June 2, 2026 13:09
@kuk1song

kuk1song commented Jun 8, 2026

Copy link
Copy Markdown
Author

Friendly check-in, happy to address any feedback if this needs changes.

Could a maintainer approve the CI workflow when convenient, so the test suite can verify the change end-to-end? I've kept the branch rebased and green locally (full gateway suite + the 12 new mention_patterns cases).

Thanks for considering!

@hangpark

hangpark commented Jun 10, 2026

Copy link
Copy Markdown

This patch is very important to me. mention_patterns is already documented in the Slack configuration documentation, but it is not actually implemented in the codebase.

The documented behavior and the actual behavior are currently out of sync. I believe this should be addressed as soon as possible to keep the documentation and implementation consistent.

@teknium1

Copy link
Copy Markdown
Contributor

Closing in favor of #50843, which shipped this same fix. Your PR (#35403) was the earliest submission for this issue (May 30) — credit to you for catching it first, @kuk1song. It targeted the old gateway/platforms/slack.py path, which has since been migrated to the plugins/platforms/slack/adapter.py plugin, so the merged version is on the current path. Thanks!

@teknium1 teknium1 closed this Jun 22, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

comp/gateway Gateway runner, session dispatch, delivery P2 Medium — degraded but workaround exists platform/slack Slack app adapter type/feature New feature or request

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants