feat(slack): add ignored channel gate - #46925
Conversation
teknium1
left a comment
There was a problem hiding this comment.
Thanks for the focused Slack control and documentation work. The requested gate is still absent on current main, but this branch predates Slack's plugin migration and needs a targeted port.
Problems
gateway/platforms/slack.pyand its legacygateway/config.pybridge were moved by1a38066054752d601b71fc655a3ed6bf4228e2da. Current routing isplugins/platforms/slack/adapter.py:2835; its config bridge is_apply_yaml_config()at:4485.- The logging change leaves
reply_to_textcontent in the same inbound INFO record (gateway/run.py:10777-10783). - Current Slack treats MPIMs as shared surfaces (
plugins/platforms/slack/adapter.py:2835), so the salvaged ignored-channel tests need MPIM coverage as well as 1:1 DMs.
Suggested changes
- Port the gate and YAML-to-env bridge into
plugins/platforms/slack/adapter.py, then add current-path tests for config/env precedence, channels, MPIMs, and 1:1 DMs. - If retaining the privacy change, remove or length-only log both message and reply-to bodies.
Automated hermes-sweeper review.
| # Check ignored channels first — these are passive/archive-only | ||
| # channels where the live bot must never respond, even to allowed | ||
| # users or explicit mentions. | ||
| ignored_channels = self._slack_ignored_channels() |
There was a problem hiding this comment.
Current main moved Slack to plugins/platforms/slack/adapter.py in 1a38066054752d601b71fc655a3ed6bf4228e2da; port this gate there. Preserve the current not is_one_to_one_dm scope so MPIMs remain subject to the control, and add an MPIM regression case.
| logger.info( | ||
| "inbound message: platform=%s user=%s chat=%s msg=%r", | ||
| "inbound message: platform=%s user=%s chat=%s msg_len=%s", | ||
| _platform_name, source.user_name or source.user_id or "unknown", |
There was a problem hiding this comment.
This replaces the main message preview, but the same log record still emits _reply_txt as reply_to_text=%r. If this change is intended to avoid inbound content in gateway logs, convert that field to a length or remove it too.
…g path Follow-up to the #51899 pick, folding in the config-bridge half of the competing PR #46925 (@bhanusharma, earliest submitter for the ignored-channel gate): - _apply_yaml_config: translate config.yaml slack.ignored_channels into SLACK_IGNORED_CHANNELS (list or CSV), env-var-wins like every other bridged Slack key. - SlackAdapter._slack_ignored_channels / gateway.run's _slack_ignored_channels_from_gateway_config: fall back to the SLACK_IGNORED_CHANNELS env var when PlatformConfig.extra carries no value, so top-level slack: blocks (which flow through the env bridge, not extra) are honored at both the adapter and runner gates. - conftest: force-clear SLACK_ALLOWED_CHANNELS / SLACK_IGNORED_CHANNELS / SLACK_DISABLE_DMS between tests (config-loader side-effect leak class). - Tests: env-bridge translation + precedence in test_config.py, env fallback + extra-wins in test_slack_runner_ignored_channels.py. Credit: #46925 by @bhanusharma proposed the same gate with the YAML→env bridge; #51899 (picked as the base) carries the wider outbound/runner coverage. Closes #46925 as consolidated here with first-submitter credit.
…g path Follow-up to the #51899 pick, folding in the config-bridge half of the competing PR #46925 (@bhanusharma, earliest submitter for the ignored-channel gate): - _apply_yaml_config: translate config.yaml slack.ignored_channels into SLACK_IGNORED_CHANNELS (list or CSV), env-var-wins like every other bridged Slack key. - SlackAdapter._slack_ignored_channels / gateway.run's _slack_ignored_channels_from_gateway_config: fall back to the SLACK_IGNORED_CHANNELS env var when PlatformConfig.extra carries no value, so top-level slack: blocks (which flow through the env bridge, not extra) are honored at both the adapter and runner gates. - conftest: force-clear SLACK_ALLOWED_CHANNELS / SLACK_IGNORED_CHANNELS / SLACK_DISABLE_DMS between tests (config-loader side-effect leak class). - Tests: env-bridge translation + precedence in test_config.py, env fallback + extra-wins in test_slack_runner_ignored_channels.py. Credit: #46925 by @bhanusharma proposed the same gate with the YAML→env bridge; #51899 (picked as the base) carries the wider outbound/runner coverage. Closes #46925 as consolidated here with first-submitter credit.
…g path Follow-up to the #51899 pick, folding in the config-bridge half of the competing PR #46925 (@bhanusharma, earliest submitter for the ignored-channel gate): - _apply_yaml_config: translate config.yaml slack.ignored_channels into SLACK_IGNORED_CHANNELS (list or CSV), env-var-wins like every other bridged Slack key. - SlackAdapter._slack_ignored_channels / gateway.run's _slack_ignored_channels_from_gateway_config: fall back to the SLACK_IGNORED_CHANNELS env var when PlatformConfig.extra carries no value, so top-level slack: blocks (which flow through the env bridge, not extra) are honored at both the adapter and runner gates. - conftest: force-clear SLACK_ALLOWED_CHANNELS / SLACK_IGNORED_CHANNELS / SLACK_DISABLE_DMS between tests (config-loader side-effect leak class). - Tests: env-bridge translation + precedence in test_config.py, env fallback + extra-wins in test_slack_runner_ignored_channels.py. Credit: #46925 by @bhanusharma proposed the same gate with the YAML→env bridge; #51899 (picked as the base) carries the wider outbound/runner coverage. Closes #46925 as consolidated here with first-submitter credit.
…g path Follow-up to the NousResearch#51899 pick, folding in the config-bridge half of the competing PR NousResearch#46925 (@bhanusharma, earliest submitter for the ignored-channel gate): - _apply_yaml_config: translate config.yaml slack.ignored_channels into SLACK_IGNORED_CHANNELS (list or CSV), env-var-wins like every other bridged Slack key. - SlackAdapter._slack_ignored_channels / gateway.run's _slack_ignored_channels_from_gateway_config: fall back to the SLACK_IGNORED_CHANNELS env var when PlatformConfig.extra carries no value, so top-level slack: blocks (which flow through the env bridge, not extra) are honored at both the adapter and runner gates. - conftest: force-clear SLACK_ALLOWED_CHANNELS / SLACK_IGNORED_CHANNELS / SLACK_DISABLE_DMS between tests (config-loader side-effect leak class). - Tests: env-bridge translation + precedence in test_config.py, env fallback + extra-wins in test_slack_runner_ignored_channels.py. Credit: NousResearch#46925 by @bhanusharma proposed the same gate with the YAML→env bridge; NousResearch#51899 (picked as the base) carries the wider outbound/runner coverage. Closes NousResearch#46925 as consolidated here with first-submitter credit.
Summary
ignored_channelssupport viaslack.ignored_channelsandSLACK_IGNORED_CHANNELS.Test plan
venv/bin/python -m pytest tests/gateway/test_slack_mention.py -q -o 'addopts='venv/bin/python -m pytest tests/gateway/test_slack.py tests/gateway/test_slack_mention.py tests/hermes_cli/test_slack_cli.py -q -o 'addopts='Notes
ignored_channels.ignored_channelsintentionally has higher precedence thanallowed_channelsandfree_response_channelsfor channel messages.