From 4044c4a00f6ae6cd238f6510fa00074a7a8acfdf Mon Sep 17 00:00:00 2001 From: sangwon Date: Tue, 26 May 2026 17:22:12 +0900 Subject: [PATCH] fix(tools): forward thread_ts to chat.postMessage so 3-segment Slack targets thread MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit send_message(target='slack::') 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:) 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. --- tests/tools/test_send_message_tool.py | 91 +++++++++++++++++++++++++++ tools/send_message_tool.py | 14 ++++- 2 files changed, 102 insertions(+), 3 deletions(-) diff --git a/tests/tools/test_send_message_tool.py b/tests/tools/test_send_message_tool.py index 66aab5eee74c8..a310a37be1dec 100644 --- a/tests/tools/test_send_message_tool.py +++ b/tests/tools/test_send_message_tool.py @@ -590,8 +590,99 @@ def test_slack_messages_are_formatted_before_send(self, monkeypatch): "***", "C123", "*hello* from ", + thread_id=None, + ) + + def test_slack_thread_id_forwarded_to_send_slack(self, monkeypatch): + """3-segment Slack targets must reach _send_slack with thread_id set + so chat.postMessage receives thread_ts and the message threads.""" + _ensure_slack_mock(monkeypatch) + import gateway.platforms.slack as slack_mod + + monkeypatch.setattr(slack_mod, "SLACK_AVAILABLE", True) + send = AsyncMock(return_value={"success": True, "message_id": "1"}) + + with patch("tools.send_message_tool._send_slack", send): + result = asyncio.run( + _send_to_platform( + Platform.SLACK, + SimpleNamespace(enabled=True, token="***", extra={}), + "C123", + "hello in thread", + thread_id="1779330010.215119", + ) + ) + + assert result["success"] is True + send.assert_awaited_once_with( + "***", + "C123", + "hello in thread", + thread_id="1779330010.215119", ) + def test_slack_send_slack_includes_thread_ts_in_payload(self, monkeypatch): + """_send_slack must put thread_ts on the chat.postMessage payload + when thread_id is provided.""" + from tools.send_message_tool import _send_slack + + captured = {} + + class _FakeResp: + async def __aenter__(self): return self + async def __aexit__(self, *a): return False + async def json(self): return {"ok": True, "ts": "9999.0001"} + + class _FakeSession: + def __init__(self, *a, **kw): pass + async def __aenter__(self): return self + async def __aexit__(self, *a): return False + def post(self, url, headers=None, json=None, **kw): + captured["url"] = url + captured["payload"] = json + return _FakeResp() + + import aiohttp + monkeypatch.setattr(aiohttp, "ClientSession", _FakeSession) + + result = asyncio.run( + _send_slack("xoxb-token", "C123", "hi", thread_id="1779330010.215119") + ) + + assert result["success"] is True + assert captured["url"] == "https://slack.com/api/chat.postMessage" + assert captured["payload"]["channel"] == "C123" + assert captured["payload"]["text"] == "hi" + assert captured["payload"]["thread_ts"] == "1779330010.215119" + + def test_slack_send_slack_omits_thread_ts_when_no_thread_id(self, monkeypatch): + """Without thread_id, _send_slack must not put a thread_ts key on the + payload (so the message lands at the channel root as intended).""" + from tools.send_message_tool import _send_slack + + captured = {} + + class _FakeResp: + async def __aenter__(self): return self + async def __aexit__(self, *a): return False + async def json(self): return {"ok": True, "ts": "9999.0001"} + + class _FakeSession: + def __init__(self, *a, **kw): pass + async def __aenter__(self): return self + async def __aexit__(self, *a): return False + def post(self, url, headers=None, json=None, **kw): + captured["payload"] = json + return _FakeResp() + + import aiohttp + monkeypatch.setattr(aiohttp, "ClientSession", _FakeSession) + + result = asyncio.run(_send_slack("xoxb-token", "C123", "hi")) + + assert result["success"] is True + assert "thread_ts" not in captured["payload"] + def test_slack_bold_italic_formatted_before_send(self, monkeypatch): """Bold+italic ***text*** survives tool-layer formatting.""" _ensure_slack_mock(monkeypatch) diff --git a/tools/send_message_tool.py b/tools/send_message_tool.py index 0f83e40c3c96e..4027fd6ce16f4 100644 --- a/tools/send_message_tool.py +++ b/tools/send_message_tool.py @@ -752,7 +752,7 @@ async def _send_to_platform(platform, pconfig, chat_id, message, thread_id=None, last_result = None for chunk in chunks: if platform == Platform.SLACK: - result = await _send_slack(pconfig.token, chat_id, chunk) + result = await _send_slack(pconfig.token, chat_id, chunk, thread_id=thread_id) elif platform == Platform.WHATSAPP: result = await _send_whatsapp(pconfig.extra, chat_id, chunk) elif platform == Platform.SIGNAL: @@ -1036,8 +1036,14 @@ async def _send_telegram(token, chat_id, message, media_files=None, thread_id=No return _error(f"Telegram send failed: {e}") -async def _send_slack(token, chat_id, message): - """Send via Slack Web API.""" +async def _send_slack(token, chat_id, message, thread_id=None): + """Send via Slack Web API. + + When ``thread_id`` is provided it is forwarded as ``thread_ts`` so the + message threads under the given parent. Without this, 3-segment targets + like ``slack::`` silently degrade to channel-root + posts. + """ try: import aiohttp except ImportError: @@ -1050,6 +1056,8 @@ async def _send_slack(token, chat_id, message): headers = {"Authorization": f"Bearer {token}", "Content-Type": "application/json"} async with aiohttp.ClientSession(timeout=aiohttp.ClientTimeout(total=30), **_sess_kw) as session: payload = {"channel": chat_id, "text": message, "mrkdwn": True} + if thread_id: + payload["thread_ts"] = str(thread_id) async with session.post(url, headers=headers, json=payload, **_req_kw) as resp: data = await resp.json() if data.get("ok"):