merge: PR #1784 refine Slack thread handling

nanobot/channels/slack.py

@@ -81,8 +81,8 @@ class SlackChannel(BaseChannel):
             slack_meta = msg.metadata.get("slack", {}) if msg.metadata else {}
             thread_ts = slack_meta.get("thread_ts")
             channel_type = slack_meta.get("channel_type")
-            # Only reply in thread for channel/group messages; DMs don't use threads
-            thread_ts_param = thread_ts if use_thread else None
+            # Slack DMs don't use threads; channel/group replies may keep thread_ts.
+            thread_ts_param = thread_ts if thread_ts and channel_type != "im" else None
 
             # Slack rejects empty text payloads. Keep media-only messages media-only,
             # but send a single blank message when the bot has no text or files to send.
@@ -278,4 +278,3 @@ class SlackChannel(BaseChannel):
             if parts:
                 rows.append(" · ".join(parts))
         return "\n".join(rows)
-

tests/test_slack_channel.py

@@ -0,0 +1,90 @@
+from __future__ import annotations
+
+import pytest
+
+from nanobot.bus.events import OutboundMessage
+from nanobot.bus.queue import MessageBus
+from nanobot.channels.slack import SlackChannel
+from nanobot.config.schema import SlackConfig
+
+
+class _FakeAsyncWebClient:
+    def __init__(self) -> None:
+        self.chat_post_calls: list[dict[str, object | None]] = []
+        self.file_upload_calls: list[dict[str, object | None]] = []
+
+    async def chat_postMessage(
+        self,
+        *,
+        channel: str,
+        text: str,
+        thread_ts: str | None = None,
+    ) -> None:
+        self.chat_post_calls.append(
+            {
+                "channel": channel,
+                "text": text,
+                "thread_ts": thread_ts,
+            }
+        )
+
+    async def files_upload_v2(
+        self,
+        *,
+        channel: str,
+        file: str,
+        thread_ts: str | None = None,
+    ) -> None:
+        self.file_upload_calls.append(
+            {
+                "channel": channel,
+                "file": file,
+                "thread_ts": thread_ts,
+            }
+        )
+
+
+@pytest.mark.asyncio
+async def test_send_uses_thread_for_channel_messages() -> None:
+    channel = SlackChannel(SlackConfig(enabled=True), MessageBus())
+    fake_web = _FakeAsyncWebClient()
+    channel._web_client = fake_web
+
+    await channel.send(
+        OutboundMessage(
+            channel="slack",
+            chat_id="C123",
+            content="hello",
+            media=["/tmp/demo.txt"],
+            metadata={"slack": {"thread_ts": "1700000000.000100", "channel_type": "channel"}},
+        )
+    )
+
+    assert len(fake_web.chat_post_calls) == 1
+    assert fake_web.chat_post_calls[0]["text"] == "hello\n"
+    assert fake_web.chat_post_calls[0]["thread_ts"] == "1700000000.000100"
+    assert len(fake_web.file_upload_calls) == 1
+    assert fake_web.file_upload_calls[0]["thread_ts"] == "1700000000.000100"
+
+
+@pytest.mark.asyncio
+async def test_send_omits_thread_for_dm_messages() -> None:
+    channel = SlackChannel(SlackConfig(enabled=True), MessageBus())
+    fake_web = _FakeAsyncWebClient()
+    channel._web_client = fake_web
+
+    await channel.send(
+        OutboundMessage(
+            channel="slack",
+            chat_id="D123",
+            content="hello",
+            media=["/tmp/demo.txt"],
+            metadata={"slack": {"thread_ts": "1700000000.000100", "channel_type": "im"}},
+        )
+    )
+
+    assert len(fake_web.chat_post_calls) == 1
+    assert fake_web.chat_post_calls[0]["text"] == "hello\n"
+    assert fake_web.chat_post_calls[0]["thread_ts"] is None
+    assert len(fake_web.file_upload_calls) == 1
+    assert fake_web.file_upload_calls[0]["thread_ts"] is None