Skip to content

fix(slack): handle bot alert Block Kit text - #52219

Closed
Sungmin-agmo wants to merge 1 commit into
NousResearch:mainfrom
Sungmin-agmo:fix/slack-bot-block-kit-alerts
Closed

fix(slack): handle bot alert Block Kit text#52219
Sungmin-agmo wants to merge 1 commit into
NousResearch:mainfrom
Sungmin-agmo:fix/slack-bot-block-kit-alerts

Conversation

@Sungmin-agmo

@Sungmin-agmo Sungmin-agmo commented Jun 25, 2026

Copy link
Copy Markdown

Summary

  • Treat free_response_channels as an explicit opt-in for external Slack bot/app messages so alert channels can trigger the agent without requiring a mention.
  • Extract readable text from Slack Block Kit section, header, fields, context, input, and actions blocks when top-level text is empty.
  • Include attachment-level Block Kit blocks and attachment fields in inbound Slack message text, covering Grafana/Alertmanager-style alert payloads.
  • Preserve self-bot loop prevention, allowed-channel gating, mention gating outside free-response channels, and existing rich-text quote extraction.

Why

Slack alert apps can emit bot_message events with empty top-level text while the human-visible payload is only present in blocks or attachments[].blocks. Those events were received by the Slack gateway but could be dropped before becoming inbound Hermes messages.

Tests

  • python -m pytest tests/gateway/test_slack_mention.py — 66 passed, 2 warnings
  • python -m pytest tests/gateway/test_slack*.py — 335 passed, 51 warnings (existing async mock/socket watchdog warnings)
  • python -m compileall -q plugins/platforms/slack/adapter.py tests/gateway/test_slack_mention.py
  • git diff --check

@Sungmin-agmo
Sungmin-agmo force-pushed the fix/slack-bot-block-kit-alerts branch from 999c28f to a7ed068 Compare June 25, 2026 01:03
@alt-glitch alt-glitch added type/feature New feature or request comp/gateway Gateway runner, session dispatch, delivery platform/slack Slack app adapter P2 Medium — degraded but workaround exists sweeper:risk-message-delivery Sweeper risk: may drop, duplicate, misroute, or suppress messages labels Jun 25, 2026
@Sungmin-agmo

Copy link
Copy Markdown
Author

Closing: this PR was opened against the receiver side by mistake. The intended sender-side fix is now in AGMO-Inc/sdm-infra#356.

@Sungmin-agmo

Copy link
Copy Markdown
Author

Reopening after producer-side validation: Slack/Grafana webhook now emits top-level text, but external Slack app events still stop at raw bot_message logs and do not reach gateway inbound handling unless receiver accepts bot messages in explicitly configured free-response alert channels.\n\nVerification rerun locally:\n- python -m pytest tests/gateway/test_slack_mention.py: 66 passed\n- python -m pytest tests/gateway/test_slack*.py: 335 passed\n- python -m compileall -q plugins/platforms/slack/adapter.py tests/gateway/test_slack_mention.py: passed

@teknium1

Copy link
Copy Markdown
Contributor

Thanks for tracing the receiver-side Slack alert path. The live inbound premise is still present on current main: plugins/platforms/slack/adapter.py:2600-2610 rejects bot events before Block Kit/attachment extraction.

Problems

  • The proposed free-response bypass is not end-to-end. Slack bot events are marked is_bot=True at plugins/platforms/slack/adapter.py:3171-3175, while gateway/authz_mixin.py:345-363 independently requires SLACK_ALLOW_BOTS=mentions or all. Thus a free-response alert channel alone is still denied after handle_message.
  • PR commit a7ed068c256aa0cb7037532716b9548a18090361 tests the adapter with a stubbed handle_message, so it does not cover that authorization boundary.
  • The same empty-text payload class remains in thread context at plugins/platforms/slack/adapter.py:3788-3790 and parent lookup at :3891-3894; the cross-referenced fix(slack): read thread context from message attachments and blocks #61261 covers that sibling case.

Suggested changes

  • Define and test a single explicit admission rule shared by adapter routing and GatewayRunner authorization, preserving self-loop and allowed_channels guards.
  • Reuse one Block Kit/attachment text helper for live messages and thread-context fetches.

Automated hermes-sweeper review.

@teknium1 teknium1 added sweeper:risk-security-boundary Sweeper risk: may affect sandboxing, auth, credentials, or sensitive data sweeper:risk-compatibility Sweeper risk: may break existing users, config, migrations, defaults, or upgrades sweeper:blast-moderate Sweeper blast radius: moderate — a subsystem or single platform labels Jul 15, 2026
@teknium1

Copy link
Copy Markdown
Contributor

Closing as superseded by #69316 (merged): block extraction landed; the bundled free_response_channels behavior change is out of this bug-class scope — feel free to re-file it standalone.

Thanks for digging into this — the consolidated fix stands on the cluster's collective analysis, and your work is credited in #69316's summary.

@teknium1 teknium1 closed this Jul 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 sweeper:blast-moderate Sweeper blast radius: moderate — a subsystem or single platform sweeper:risk-compatibility Sweeper risk: may break existing users, config, migrations, defaults, or upgrades sweeper:risk-message-delivery Sweeper risk: may drop, duplicate, misroute, or suppress messages sweeper:risk-security-boundary Sweeper risk: may affect sandboxing, auth, credentials, or sensitive data type/feature New feature or request

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants