fix: Slack thread routing — extend _SLACK_TARGET_RE to capture thread_ts - #24332
Closed
dirtyren wants to merge 5 commits into
Closed
fix: Slack thread routing — extend _SLACK_TARGET_RE to capture thread_ts#24332dirtyren wants to merge 5 commits into
dirtyren wants to merge 5 commits into
Conversation
Extend send_message_tool to support MEDIA:<path> attachments on Slack, using the modern two-step upload API (files.getUploadURLExternal + files.completeUploadExternal) with automatic fallback to the legacy files.upload endpoint. Changes: - _send_slack: add thread_id parameter (was silently dropped before) - _send_slack_file: new helper — modern 2-step upload, MIME detection, thread reply support, retryable via 3-attempt outer loop - _send_slack_file_legacy: fallback for workspaces where the modern endpoint returns ok=false - _send_to_platform: add Slack MEDIA block (mirrors Discord/Matrix pattern) — text chunks sent first, files uploaded on the last chunk - Error strings and tool description updated to include slack in the list of supported MEDIA platforms Requires the Slack bot to have the files:write OAuth scope. Tests (8 new, all passing): - TestSendSlackFile: missing file, modern happy path, thread_id forwarding, legacy fallback - TestSendToPlatformSlackMedia: media-only, text+media, thread_id forwarded to both helpers, file error propagation
…nel_created
Add SlackAutoJoinConfig dataclass (gateway/config.py) with two knobs:
- enabled (bool, default false) — feature toggle
- channel_regex (str, default empty) — regex applied to channel name
When a channel_created event is received and enabled=true:
1. Compile regex at connect() time with re.IGNORECASE; invalid regex
logs a warning and disables the feature (no crash).
2. If the new channel name matches, call conversations_join.
3. On success, populate _channel_team for multi-workspace routing.
4. missing_scope / not_allowed errors produce an actionable ERROR log
directing the operator to add channels:join to the Slack app manifest.
5. Mismatch produces DEBUG log only.
Config schema (config.yaml top-level key):
slack_auto_join:
enabled: true
channel_regex: "^inc-.*$"
Required Slack app manifest additions:
- event_subscriptions.bot_events: channel_created
- oauth_config.scopes.bot: channels:join, channels:read
Add a top-level `allow_all_users` boolean to config.yaml (default: false) that grants open access to all users regardless of per-platform allowlists. Also adds `slack.allow_all_users_on_channel` for channel-scoped bypass on Slack — any message from a listed channel skips the per-user check. Changes: - hermes_cli/config.py: add `allow_all_users: false` to DEFAULT_CONFIG - gateway/config.py: add `allow_all_users: bool = False` field to GatewayConfig, parse in from_dict(), map from config.yaml loader - gateway/run.py: check config.allow_all_users as first real auth step in _is_user_authorized() before all allowlist logic; update docstring - gateway/platforms/slack.py: add _slack_allow_all_users_on_channel() helper + startup warning when feature is enabled
Add slack_prompt_on_join to SlackAutoJoinConfig so the bot can fire a
configurable agent prompt immediately after joining a channel whose name
matches channel_regex.
Changes:
- gateway/config.py: add slack_prompt_on_join field (default "") to
SlackAutoJoinConfig; update to_dict() and from_dict(); update docstring
to document member_joined_channel requirement.
- gateway/platforms/slack.py:
- Store _auto_join_prompt on adapter init.
- Read slack_prompt_on_join from config at connect() time.
- _handle_channel_created(): track join success; call
_dispatch_prompt_on_join() when joined and prompt is set.
- _handle_member_joined_channel() (new): fires when the bot is manually
invited to an existing channel; guards on bot user_id match, resolves
channel name via API when absent, checks regex, then calls
_dispatch_prompt_on_join(). Registered via @app.event.
- _dispatch_prompt_on_join() (new): substitutes ${channel_name} in the
template, builds a synthetic internal MessageEvent, and routes it
through handle_message() (bypassing user-auth via internal=True).
Config example (config.yaml):
slack_auto_join:
enabled: true
channel_regex: "^inc-.*$"
slack_prompt_on_join: "investigate ${channel_name} with skill investigate-p0"
The internal=True flag on the synthetic MessageEvent ensures the bot-user
that triggers the event is not rejected by the allowlist check.
Three bugs in tools/send_message_tool.py caused every Slack thread reply to
fail silently and fall back to wrong-channel or top-level posts:
Bug 1: _SLACK_TARGET_RE only matched bare channel IDs — rejected CHANNEL:TS
format entirely, returning (None, None, False) → home-channel fallback.
Bug 2: _parse_target_ref hardcoded None for Slack thread_id even if regex matched.
Bug 3: tool description didn't mention Slack thread format as a valid example.
Fixes:
- Extend _SLACK_TARGET_RE to r'^\s*([CGD][A-Z0-9]{8,})(?:([\d]+\.[\d]+))?\s*$'
(optional decimal thread_ts group — Slack uses epoch.seq format)
- _parse_target_ref returns match.group(2) instead of hardcoded None for Slack
- Update tool 'target' description to include slack:CHANNEL:TS example
Also adds structural guard in gateway/session.py build_session_context_prompt():
when source.thread_id is set for Slack, inject explicit IMPORTANT directive
telling the agent to send all responses to slack:CHANNEL:TS — fires even if
SOUL.md is ever overwritten.
Tests: all 20 Slack send_message tests pass (updated one assert to include
thread_id=None kwarg that now propagates through _send_to_platform).
alt-glitch
added
type/bug
Collaborator
Contributor
|
Automated hermes-sweeper review: this Slack thread-routing fix is already implemented on current Evidence:
Also acknowledging the prior maintainer note: this PR was identified as a duplicate of the canonical Slack thread-routing fix lineage (#16992 and related PRs). |
teknium1
added
the
sweeper:implemented-on-main
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
3 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.
Problem
Three bugs in
tools/send_message_tool.pycaused every Slack thread reply to fail silently and cascade into wrong-channel or top-level posts:Bug 1 —
_SLACK_TARGET_RErejectedCHANNEL:TSformat (primary)The regex
r"^\s*([CGD][A-Z0-9]{8,})\s*$"only matched bare channel IDs. Whentarget_ref = "C0ATFHY907L:1778251420.873289",fullmatchfailed →(None, None, False)→ fell through to channel-name resolution → wrong channel or home channel.Bug 2 —
_parse_target_refhardcodedNonefor Slackthread_idEven if the regex had matched,
return match.group(1), None, Truediscarded the thread timestamp.Bug 3 — Tool description didn't document Slack thread format
The
targetdescription listed Telegram/Discord thread examples but not Slack.Fix
tools/send_message_tool.py_SLACK_TARGET_REto capture optional decimal thread_ts:_parse_target_refreturnsmatch.group(2)instead of hardcodedNonetargetdescription updated withslack:C0ATFHY907L:1778251420.873289examplegateway/session.pyWhen
source.platform == Platform.SLACKandsource.thread_idis set, inject an explicit IMPORTANT directive intobuild_session_context_prompt()telling the agent to reply toslack:CHANNEL:TS. Structural backup — fires even if SOUL.md is overwritten.Test Results
All 20 Slack send_message tests pass (updated one assert to include
thread_id=Nonekwarg).