Skip to content

fix: Slack thread routing — parse channel_id:thread_ts in send_message target - #21486

Closed
dirtyren wants to merge 1 commit into
NousResearch:mainfrom
dirtyren:fix/slack-thread-routing
Closed

fix: Slack thread routing — parse channel_id:thread_ts in send_message target#21486
dirtyren wants to merge 1 commit into
NousResearch:mainfrom
dirtyren:fix/slack-thread-routing

Conversation

@dirtyren

@dirtyren dirtyren commented May 7, 2026

Copy link
Copy Markdown
Contributor

Problem

send_message(target='slack:C0B0GV4FKK7:1778181769.602679') was silently routing to the home channel (C0ATFHY907L) instead of channel C0B0GV4FKK7 in the specified thread.

Root Cause

Three separate bugs in tools/send_message_tool.py:

Bug 1 — _parse_target_ref regex miss (primary)
_SLACK_TARGET_RE = r'^\s*([CGD][A-Z0-9]{8,})\s*$' only matches bare channel IDs.
When target_ref = "C0B0GV4FKK7:1778181769.602679", the regex didn't match → returned (None, None, False) → fell through to channel-name resolution → fell back to home channel.

Bug 2 — _send_slack() missing thread_id param
Even if parsing had worked, _send_slack() had no thread_id parameter and never set thread_ts in the Slack API payload.

Bug 3 — _send_to_platform() not forwarding thread_id
The dispatch call await _send_slack(pconfig.token, chat_id, chunk) dropped thread_id entirely.

Fix

  1. Added _SLACK_THREAD_TARGET_RE = re.compile(r'^\s*([CGD][A-Z0-9]{8,})':([\d.]+)\s*$') — handles CHANNEL_ID:THREAD_TS format (Slack timestamps use decimal notation like 1778181769.602679).
  2. Updated _parse_target_ref to try the thread regex first, then fall back to bare-channel regex (no breakage of existing behaviour).
  3. Added thread_id=None param to _send_slack() and inject payload["thread_ts"] = thread_id when set.
  4. Passed thread_id=thread_id in _send_to_platform() dispatch.

Tests

  • Updated existing assert_awaited_once_with assertion to include thread_id=None keyword arg.
  • Added test_channel_with_thread_ts_is_parsed — verifies C0B0GV4FKK7:1778181769.602679 parses to (chat_id="C0B0GV4FKK7", thread_id="1778181769.602679", is_explicit=True).
  • Added test_bare_channel_still_works_after_thread_regex — regression guard for bare channel ID path.

All 97 tests pass (pytest tests/tools/test_send_message_tool.py).

…e target

When send_message is called with target='slack:CHANNEL_ID:THREAD_TS',
_parse_target_ref failed to match the colon-separated format, falling
back to home channel instead. Two bugs:

1. _SLACK_TARGET_RE only matched bare channel IDs (no :thread_ts).
   Added _SLACK_THREAD_TARGET_RE = r'([CGD][A-Z0-9]{8,}):([\d.]+)'
   and try it first before the bare-channel regex.

2. _send_slack() had no thread_id parameter, so even if parsing worked,
   the thread_ts was never sent to Slack's chat.postMessage API.
   Added thread_id=None param and inject payload['thread_ts'] when set.

3. _send_to_platform() was calling _send_slack() without thread_id.
   Now passes thread_id=thread_id.

Reproducer: send_message(target='slack:C0B0GV4FKK7:1778181769.602679')
was silently routing to home channel C0ATFHY907L.
@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 May 7, 2026
@alt-glitch

Copy link
Copy Markdown
Collaborator

Duplicate of #16992 (still open) — same fix: parse channel_id:thread_ts in Slack send_message target and plumb thread_ts through to the Slack API payload. Prior PRs #14146, #16962, #17741, #18911 all addressed the same issue.

@teknium1

Copy link
Copy Markdown
Contributor

This appears to be implemented on current main now. Thanks for the useful duplicate fix and writeup — this automated hermes-sweeper review found the same behavior already landed.

Evidence:

  • tools/send_message_tool.py:31 defines _SLACK_THREAD_TARGET_RE for Slack <conversation_id>:<thread_ts> targets.
  • tools/send_message_tool.py:368 parses Slack thread targets before bare channel IDs and returns (chat_id, thread_id, True).
  • tools/send_message_tool.py:784 forwards thread_id into _send_slack(..., thread_ts=thread_id).
  • tools/send_message_tool.py:1064 / tools/send_message_tool.py:1078 make _send_slack() accept thread_ts and include it in the Slack chat.postMessage payload.
  • tests/tools/test_send_message_tool.py:1230 covers a Slack thread target parse regression.
  • The final plumbing landed in 74e845c000de1f32cd325758407ea706f18b7c36 and is contained in release tag v2026.6.5.

@teknium1 teknium1 closed this Jun 11, 2026
@teknium1 teknium1 added the sweeper:implemented-on-main Sweeper: behavior already present on current main label Jun 11, 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