fix(tools): forward thread_ts to chat.postMessage so 3-segment Slack targets thread - #32496
Closed
bejewel-sangwon wants to merge 1 commit into
Closed
fix(tools): forward thread_ts to chat.postMessage so 3-segment Slack targets thread#32496bejewel-sangwon wants to merge 1 commit into
bejewel-sangwon wants to merge 1 commit into
Conversation
…targets thread send_message(target='slack:<channel>:<thread_ts>') was silently degrading to a channel-root post because _send_to_platform did not forward thread_id to _send_slack, and _send_slack did not include thread_ts on the chat.postMessage payload. Every other platform branch (Telegram, Discord, Matrix, Feishu, Signal, Yuanbao) already forwards thread_id. Slack was the only exception. Behavior change: - Slack 3-segment targets now thread correctly under the given parent. - 2-segment targets (slack:<channel>) keep existing channel-root behavior — payload omits thread_ts when thread_id is None. Tests: - test_slack_thread_id_forwarded_to_send_slack: verifies _send_to_platform passes thread_id keyword to _send_slack. - test_slack_send_slack_includes_thread_ts_in_payload: verifies thread_ts lands on the chat.postMessage payload when set. - test_slack_send_slack_omits_thread_ts_when_no_thread_id: verifies the channel-root path stays clean when no thread_id is provided. - test_slack_messages_are_formatted_before_send: updated to assert the new thread_id=None keyword argument. All 124 tests in tests/tools/test_send_message_tool.py pass.
Author
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
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.
What does this PR do?
send_message(target="slack:<channel>:<thread_ts>")was silently posting to the channel root instead of threading under the given parent. The 3-segment Slack target syntax existed and was parsed, but thethread_tswas dropped on the way tochat.postMessage.Two consecutive layers were responsible:
_send_to_platform(tools/send_message_tool.py), the Slack branch did not forwardthread_idto_send_slack. Every other platform branch (Telegram, Discord, Matrix, Feishu, Signal, Yuanbao) does._send_slackitself did not includethread_tson thechat.postMessagepayload at all.Net effect: any caller passing
slack:<channel>:<thread_ts>got the parsing-side validation to pass but the message still landed at the channel root. This surfaced as multi-bot conversation chains "splitting" across multiple threads in environments that rely onsend_messageto keep replies inside the originating thread.Related Issue
N/A — discovered while debugging cross-bot Slack threading in a self-hosted gateway deployment.
Type of Change
Changes Made
tools/send_message_tool.py:_send_to_platformSlack branch now forwardsthread_idto_send_slack._send_slackgains athread_idparameter and, when set, includesthread_tson thechat.postMessagepayload.tests/tools/test_send_message_tool.py:test_slack_thread_id_forwarded_to_send_slacktest_slack_send_slack_includes_thread_ts_in_payloadtest_slack_send_slack_omits_thread_ts_when_no_thread_idtest_slack_messages_are_formatted_before_sendto assert the newthread_id=Nonekeyword argument.How to Test
All 124 tests pass locally (4.8s on macOS 15.x).
Manual repro of the original bug (pre-fix):
Checklist
Code
scripts/run_tests.sh tests/tools/test_send_message_tool.pyand all 124 tests passDocumentation & Housekeeping
_send_slackdocstring now documents the new parametercli-config.yaml.exampleif I added/changed config keys — N/ACONTRIBUTING.mdorAGENTS.mdif I changed architecture or workflows — N/ASEND_MESSAGE_SCHEMAdescription that already documents 3-segment targets)