Skip to content

fix(tools): add Slack channel ID parsing to send_message target resolution - #11101

Closed
HelmholtzW wants to merge 2 commits into
NousResearch:mainfrom
HelmholtzW:fix/slack-channel-id-target-resolution
Closed

fix(tools): add Slack channel ID parsing to send_message target resolution#11101
HelmholtzW wants to merge 2 commits into
NousResearch:mainfrom
HelmholtzW:fix/slack-channel-id-target-resolution

Conversation

@HelmholtzW

Copy link
Copy Markdown

What Changed

Added Slack channel ID parsing to _parse_target_ref() in tools/send_message_tool.py.

Why

_parse_target_ref had handlers for Telegram, Discord, Feishu, WeChat, and Matrix — but not Slack. When send_message was called with a Slack channel ID target (e.g. slack:C0ANYSL2GBE), the ID was not recognized as explicit, causing the message to fall through to the home channel instead of being sent to the specified channel.

Slack channel IDs start with uppercase letters indicating their type:

  • C = channel
  • D = DM
  • G = group (private channel)
  • B = bot
  • U/F/V/W = user/app identifiers

They may also include a thread timestamp after a colon (e.g. C0ANYSL2GBE:1776351448.347679).

Changes

tools/send_message_tool.py

  • Added Slack ID regex ^([BCDFGUVW][A-Z0-9]+)(?::(\d+\.\d+))?$ to _parse_target_ref
  • Returns (channel_id, thread_ts, True) for recognized Slack IDs
  • Positioned before the generic numeric fallback to prevent false matches

tests/tools/test_send_message_tool.py

  • Added TestParseTargetRefSlack class with 10 new tests:
    • Valid IDs for all prefixes (C, D, G, B, U, F, V, W)
    • Thread timestamp extraction
    • Platform specificity (Slack IDs not matched on Telegram/Discord)
    • Negative cases (lowercase, invalid prefix, malformed thread)

How to Test

source venv/bin/activate
pytest tests/tools/test_send_message_tool.py::TestParseTargetRefSlack -v

All 19 parse_target tests pass:

pytest tests/tools/test_send_message_tool.py -v -k "parse_target" -o "addopts="

Platforms Tested

  • Linux (Python 3.11.15)
  • All existing send_message tests continue to pass
  • 10 new Slack ID parsing tests pass

HelmholtzW and others added 2 commits April 16, 2026 15:35
…ution

Slack channel IDs (C=channel, D=DM, G=group, B=bot, U/F/V/W=user/app)
were not handled by _parse_target_ref. This caused send_message calls
targeting slack:<ID> to fall through to the home channel instead of
sending to the specified channel.

The fix adds a Slack-specific regex handler that:
- Recognizes uppercase-prefixed Slack IDs (C, D, G, B, F, U, V, W)
- Supports optional thread timestamp after colon (e.g. C0ANYSL2GBE:1776351448.347679)
- Returns (channel_id, thread_ts, True) for proper routing

Includes 10 new unit tests covering:
- All valid Slack ID prefixes (C, D, G, B, U, F, V, W)
- Thread timestamp extraction
- Platform specificity (Slack IDs ignored on non-Slack platforms)
- Negative cases (lowercase, invalid prefixes, malformed threads)

Platforms tested: Linux (Python 3.11), all 19 parse_target tests pass.
…nts, add whitespace handling

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
@alt-glitch alt-glitch added type/bug Something isn't working P2 Medium — degraded but workaround exists comp/tools Tool registry, model_tools, toolsets platform/slack Slack app adapter labels Apr 25, 2026
@alt-glitch

Copy link
Copy Markdown
Collaborator

Likely duplicate of #15064 — same fix: treat Slack alphanumeric channel IDs (C.../D.../G...) as explicit targets in send_message target parser. Also related to #14146 (Slack thread_ts support).

@teknium1

Copy link
Copy Markdown
Contributor

Thanks for the focused Slack target-resolution coverage. This is an automated hermes-sweeper review; current main already implements this behavior.

  • tools/send_message_tool.py:552-565 parses Slack conversation IDs and channel_id:thread_ts targets before channel-name resolution.
  • tests/tools/test_send_message_tool.py:1622-1663 covers C/G/D IDs, thread targets, whitespace, invalid targets, and platform isolation.
  • The behavior landed in 802c7acb813b9845cd2b6aefeaf193e7176908f3 and was tightened by 75d3eaa0e4b9c602933b2ad269f0ea0f593b5d2d to avoid treating invalid U/W user/workspace IDs as direct chat.postMessage conversation targets.

The related duplicate note from @alt-glitch is consistent with the implementation now on main.

@teknium1 teknium1 closed this Jul 12, 2026
@teknium1 teknium1 added the sweeper:implemented-on-main Sweeper: behavior already present on current main label Jul 12, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

comp/tools Tool registry, model_tools, toolsets P2 Medium — degraded but workaround exists platform/slack Slack app adapter sweeper:implemented-on-main Sweeper: behavior already present on current main type/bug Something isn't working

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants