fix(slack): honor disable_dms, ignored channels, threading defaults + small correctness batch - #70196
Merged
Conversation
Contributor
૮ >ﻌ< ა ci reviewran on 411d298 all good! |
alt-glitch
added
type/bug
teknium1
force-pushed
the
slack/c18-settings
branch
2 times, most recently
from
July 23, 2026 18:43
ac805fe to
aa76bcc
Compare
Blank (or whitespace-only) answers to the home-channel prompt in the interactive setup wizards previously left any previously saved *_HOME_CHANNEL / *_HOME_ROOM env value in place, so operators could not clear a stale home channel by re-running setup. Strip the prompt input and call remove_env_value() on blank answers across the Discord, Slack, Feishu, Matrix, Mattermost, WeCom and WhatsApp plugin setup wizards, with per-adapter wizard tests covering set/clear/whitespace flows. Squash of the three commits from PR #58421 (setup-wizard fix, matrix/ wecom extension, and 6-adapter test coverage) — one commit per contributor on this salvage branch. Fixes #12423 Salvaged from #58421
.get("key", {}) only applies the default when the key is ABSENT.
When the key exists with value None (null in JSON), .get() returns
None and the subsequent .get() raises AttributeError.
Fix: replace .get("key", {}).get(...) with (.get("key") or {}).get(...)
which handles both missing keys AND None values.
8 instances across 6 files:
- gateway/run.py: tool_call function name check
- acp_adapter/server.py: tool name/description extraction
- gateway/platforms/qqbot/onboard.py: API response task_id
- gateway/platforms/yuanbao.py: message content parsing (x2)
- gateway/platforms/slack.py: block text extraction (x2)
- tui_gateway/server.py: error message extraction
Hermes already identifies itself on outbound calls to model providers
and monitoring endpoints — see hermes_cli/models.py and
plugins/model-providers/gmi/__init__.py
("Attribution so GMI can identify traffic from Hermes Agent"). The
Slack adapter is the one outbound surface that doesn't carry the
same identifier, so HermesAgent-driven Slack traffic is
indistinguishable from any other Bolt-Python app at the Slack
platform layer.
This sets `user_agent_prefix=f"HermesAgent/{_HERMES_VERSION}"` on the
AsyncWebClient instances constructed in gateway/platforms/slack.py
and threads them through AsyncApp via its `client` kwarg. Both
kwargs are first-class in slack-sdk and slack-bolt; no new
dependencies. Resulting header looks like:
HermesAgent/<version> Python/3.x slackclient/3.x ...
No behavioral change for users — the Slack API ignores User-Agent
semantically; it lands in logs and analytics. Reversible.
Tests in tests/gateway/test_slack.py:
- TestSlackUserAgent pins the prefix shape and runs connect()
end-to-end (multi-token config) to assert every AsyncWebClient
carries the prefix and AsyncApp receives the pre-built client.
- TestSlackProxyBehavior fakes updated to tolerate the new kwargs
via **_kwargs so they don't break on future passthroughs.
PR #37303 added per-platform streaming defaults and the commit message explicitly called out "Discord/Slack/etc. only have edit-based streaming (repeated editMessage), which flickers and is noticeably jankier" — but only discord.streaming=false was shipped. Slack uses the same edit-based streaming mechanism and has the same flicker problem, yet it was left to follow the global switch (default true when streaming is enabled). Add "slack": {"streaming": False} to DEFAULT_CONFIG["display"]["platforms"] alongside the Discord default. The same deep-merge semantics apply: a user who explicitly sets display.platforms.slack.streaming: true keeps their value unchanged. The dashboard schema gains a slack.streaming toggle automatically since it is generated from DEFAULT_CONFIG. Update test_per_platform_streaming_defaults.py to cover slack in all existing assertions and rename the resolver test to reflect both platforms.
Add --long-description / --long-description-file to `hermes slack manifest` so the generated app manifest can carry Slack's display_information.long_description (175–4,000 characters), with validation of the length bounds, mutual-exclusion with --slashes-only, and UTF-8 file input. Also propagate the manifest command's exit status through cmd_slack so validation failures reach the shell. Squash of the two commits from PR #65256 — one commit per contributor on this salvage branch. Salvaged from #65256
…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.
Previously, Slack sessions and the channel directory stored raw channel IDs (e.g. C0ATFHY907L) as chat_name, making it impossible for operators to identify channels in send_message listings or session data. Changes: - Add _channel_name_cache and _resolve_channel_name() to SlackAdapter that calls conversations.info (channels) or users.info (DMs) with in-memory caching to avoid repeated API calls - Use resolved names in _handle_slack_message() build_source() calls - Use cached names in _seed_assistant_thread_session() - Enrich session-sourced entries in channel_directory._build_slack() by cross-referencing API results and falling back to conversations.info + users.info for DMs and private channels not in the bot's scope Fixes: channel directory and session origins now show readable names like 'general' or 'John Doe' instead of 'C0xxx' / 'D0xxx'.
…ndividually Two related Slack delivery fixes for send_message text sends: - Route Slack text delivery through _send_via_adapter so the live in-process gateway adapter (multi-workspace aware, channel→client mapping, adapter-side gates) is preferred, with the plugin's _standalone_send as the out-of-process fallback — matching how the media path already behaves. - _standalone_send: SLACK_BOT_TOKEN can be a comma-separated list in multi-workspace installs and slack_tokens.json carries OAuth per-workspace tokens; the standalone Web-API path used to send the literal comma-joined string, which Slack rejects as invalid_auth. Try each token individually, retrying on token-scoped errors (invalid_auth / not_in_channel / channel_not_found …) and stopping on terminal ones. User-DM resolution (U…/W… targets) also tries each token. Adapted from #47547 by @replygirl — the original patched the legacy tools/send_message_tool.py::_send_slack helper, which moved to the Slack plugin's _standalone_send in #41112. Salvaged from #47547
Note: #51899's commit was authored as a placeholder identity (hermes-agent@example.invalid); it was re-stamped to @byshubham's numeric noreply on this branch, so no mapping file is needed. Other salvaged commits use GitHub noreply addresses.
…annel guard Gateway tests construct GatewayRunner via object.__new__ without __init__; the new #51899 guard accessed self.config directly and crashed those runners (AttributeError). Use getattr with None default — the guard no-ops without config, matching the documented object.__new__ pitfall.
teknium1
force-pushed
the
slack/c18-settings
branch
from
July 23, 2026 18:52
aa76bcc to
411d298
Compare
This was referenced Jul 23, 2026
1 task
19 tasks
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
11 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Slack settings that were silently ignored are now honored —
disable_dms, ignored channels (gated at both runner and adapter, across send/edit/typing/uploads), surface-specific reply-threading defaults — plus a batch of small correctness fixes, each verified live against main first.Changes (per-item verdicts in the report)
disable_dmshonored in message handler + slash commands (fix(slack): honor disable_dms setting #53030).Credits
Salvaged with authorship preserved: #51899 (@byshubham — placeholder commit identity re-stamped to their noreply, flagged), #53030 (@vexclawx31), #32580 (@jackberger03), #58421 (@DavidMetcalfe), #17693, #34695 (@annguyenNous), #43734 (@mwbrooks), #65256 (@2001Y). #46925 (@bhanusharma) folded with credit.
Validation
tests/gateway/ -q -k slackInfographic