Slack reply_in_thread config option behavior and fixes

Fixes on record

Merged to main. A stamp counts the issues a fix is linked to.

• #4643 feat(slack): add reply_in_thread config option

  merged to main

  files: gateway/platforms/slack.py, website/docs/user-guide/messaging/slack.md

  Show the diff

  • gateway/platforms/slack.py

    @@ -323,7 +323,18 @@ def _resolve_thread_ts(
     
             Prefers metadata thread_id (the thread parent's ts, set by the
             gateway) over reply_to (which may be a child message's ts).
    +
    +        When ``reply_in_thread`` is ``false`` in the platform extra config,
    +        top-level channel messages receive direct channel replies instead of
    +        thread replies.  Messages that originate inside an existing thread are
    +        always replied to in-thread to preserve conversation context.
             """
    +        # When reply_in_thread is disabled (default: True for backward compat),
    +        # only thread messages that are already part of an existing thread.
    +        if not self.config.extra.get("reply_in_thread", True):
    +            existing_thread = (metadata or {}).get("thread_id") or (metadata or {}).get("thread_ts")
    +            return existing_thread or None
    +
             if metadata:
                 if metadata.get("thread_id"):
                     return metadata["thread_id"]

  • website/docs/user-guide/messaging/slack.md

    @@ -217,6 +217,23 @@ In channels, always @mention the bot. Simply typing a message without mentioning
     This is intentional — it prevents the bot from responding to every message in busy channels.
     :::
     
    +### Reply Threading
    +
    +By default, Hermes replies in a **thread** attached to the original message in channels. If your team prefers replies to go **directly to the channel** instead, you can disable threading:
    +
    +```yaml
    +platforms:
    +  slack:
    +    extra:
    +      reply_in_thread: false
    +```
    +
    +When `reply_in_thread` is `false`:
    +- **Channel messages** — Hermes replies directly in the channel (no thread created)
    +- **Thread messages** — Hermes still replies inside the existing thread to preserve conversation context
    +
    +The default is `true` (threaded replies), which matches the original behavior.
    +
     ---
     
     

• #16188 fix(slack): honor reply_in_thread=false in DMs and channels

  merged to main

  files: gateway/config.py, gateway/platforms/slack.py, tests/gateway/test_slack_mention.py

  Show the diff

  • gateway/config.py

    @@ -570,6 +570,8 @@ def load_gateway_config() -> GatewayConfig:
                         )
                     if "reply_prefix" in platform_cfg:
                         bridged["reply_prefix"] = platform_cfg["reply_prefix"]
    +                if "reply_in_thread" in platform_cfg:
    +                    bridged["reply_in_thread"] = platform_cfg["reply_in_thread"]
                     if "require_mention" in platform_cfg:
                         bridged["require_mention"] = platform_cfg["require_mention"]
                     if "free_response_channels" in platform_cfg:

  • gateway/platforms/slack.py

    @@ -450,8 +450,18 @@ def _resolve_thread_ts(
             """
             # When reply_in_thread is disabled (default: True for backward compat),
             # only thread messages that are already part of an existing thread.
    +        # For top-level channel messages, the inbound handler sets
    +        # metadata.thread_id to the message's own ts as a session-keying
    +        # fallback (see the `thread_ts = event.get("thread_ts") or ts` branch),
    +        # so metadata alone can't distinguish a real thread reply from a
    +        # top-level message. reply_to is the incoming message's own id, so
    +        # when thread_id == reply_to the "thread" is synthetic and we reply
    +        # directly in the channel instead.
             if not self.config.extra.get("reply_in_thread", True):
    -            existing_thread = (metadata or {}).get("thread_id") or (metadata or {}).get("thread_ts")
    +            md = metadata or {}
    +            existing_thread = md.get("thread_id") or md.get("thread_ts")
    +            if existing_thread and reply_to and existing_thread == reply_to:
    +                existing_thread = None
                 return existing_thread or None
     
             if metadata:

  • tests/gateway/test_slack_mention.py

    @@ -310,3 +310,43 @@ def test_config_bridges_slack_free_response_channels(monkeypatch, tmp_path):
         import os as _os
         assert _os.environ["SLACK_REQUIRE_MENTION"] == "false"
         assert _os.environ["SLACK_FREE_RESPONSE_CHANNELS"] == "C0AQWDLHY9M,C9999999999"
    +
    +
    +def test_config_bridges_slack_reply_in_thread(monkeypatch, tmp_path):
    +    from gateway.config import load_gateway_config
    +
    +    hermes_home = tmp_path / ".hermes"
    +    hermes_home.mkdir()
    +    (hermes_home / "config.yaml").write_text(
    +        "slack:\n"
    +        "  reply_in_thread: false\n",
    +        encoding="utf-8",
    +    )
    +
    +    monkeypatch.setenv("HERMES_HOME", str(hermes_home))
    +    monkeypatch.setenv("SLACK_BOT_TOKEN", "xoxb-test")
    +
    +    config = load_gateway_config()
    +
    +    assert config is not None
    +    slack_config = config.platforms[Platform.SLACK]
    +    assert slack_config.extra.get("reply_in_thread") is False
    +
    +    adapter = SlackAdapter(slack_config)
    +    assert adapter._resolve_thread_ts(reply_to="171.000", metadata={}) is None
    +
    +    # Top-level channel messages arrive with metadata.thread_id == reply_to
    +    # because the inbound handler uses event.ts as a session-keying fallback.
    +    # Those must be treated as non-threaded so reply_in_thread=false takes
    +    # effect in channels, not just DMs.
    +    assert adapter._resolve_thread_ts(
    +        reply_to="171.000",
    +        metadata={"thread_id": "171.000"},
    +    ) is None
    +
    +    # Real thread replies (reply_to differs from thread parent) must still
    +    # resolve to the parent thread so conversation context is preserved.
    +    assert adapter._resolve_thread_ts(
    +        reply_to="171.500",
    +        metadata={"thread_id": "171.000"},
    +    ) == "171.000"

• #41703 fix(slack): scope top-level channel messages by channel-only when reply_in_thread=false (#15421)

  merged to main

  files: gateway/platforms/slack.py, tests/gateway/test_slack_channel_session_scope.py

  Show the diff

  • gateway/platforms/slack.py

    @@ -2291,7 +2291,38 @@ async def _handle_slack_message(self, event: dict) -> None:
                 if not thread_ts and self._dm_top_level_threads_as_sessions():
                     thread_ts = ts
             else:
    -            thread_ts = event.get("thread_ts") or ts  # ts fallback for channels
    +            # Channel message session scoping.
    +            #
    +            # Three cases:
    +            #   (a) genuine thread reply   → scope session per thread
    +            #   (b) top-level, reply_in_thread=true (the default)  →
    +            #       legacy behaviour: each top-level message becomes its
    +            #       own thread, so the UX still "replies in a thread"
    +            #       and sessions are keyed per thread root
    +            #   (c) top-level, reply_in_thread=false → scope one session
    +            #       across the whole channel so context accumulates across
    +            #       messages (#15421 bug 1)
    +            event_thread_ts_raw = event.get("thread_ts")
    +            # Align with ``is_thread_reply`` below — a ``thread_ts ==
    +            # ts`` payload (some thread-root shapes) is not a real reply
    +            # and must not prevent the shared-session path from taking
    +            # effect.  Matching the same invariant here keeps the two
    +            # branches in sync even if Slack introduces new payload
    +            # variants (Copilot on #15464).
    +            if event_thread_ts_raw and event_thread_ts_raw != ts:
    +                thread_ts = event_thread_ts_raw
    +            elif self.config.extra.get("reply_in_thread", True):
    +                # Legacy default: treat ts as a synthetic thread root so
    +                # this top-level message gets its own session.
    +                thread_ts = ts
    +            else:
    +                # reply_in_thread=false: no thread key → session manager
    +                # groups by (platform, channel_id, None) and the channel
    +                # shares one conversation.  reply_to_message_id at the
    +                # outbound side is already gated on ``thread_ts != ts``
    +                # so None here produces a non-threaded reply without
    +                # further changes.
    +                thread_ts = None
     
             # In channels, respond if:
             #   0. Channel is in free_response_channels, OR require_mention is

  • tests/gateway/test_slack_channel_session_scope.py

    @@ -0,0 +1,259 @@
    +"""Regression guard for #15421 bug 1 — Slack channel session scoping.
    +
    +Before this fix, every top-level Slack channel message got a unique
    +``thread_id`` (the message's own ``ts``) stamped onto its
    +``MessageSource``.  The gateway session store keys sessions by
    +``(platform, channel_id, thread_id)``, so each top-level message
    +spawned a **brand new session** and channel context never accumulated
    +across messages — even when the operator set ``reply_in_thread: false``
    +in ``config.yaml`` expecting channel-wide conversation.
    +
    +The fix: when ``reply_in_thread: false`` is configured, top-level
    +channel messages now land on ``thread_id = None`` so the session store
    +groups them under a single channel-scoped session.  Genuine thread
    +replies (``event.thread_ts != ts``) still scope sessions per thread in
    +both modes — threading UX is unchanged when the operator actually
    +asks for it.
    +
    +These tests drive the real ``SlackAdapter._handle_slack_message`` code
    +path with mocked aiohttp / user-resolution so the ``MessageEvent``
    +that reaches ``handle_message`` exposes exactly what the session store
    +will key on.  Asserting on the event keeps the seam tight against the
    +production function's behaviour rather than a re-implementation.
    +"""
    +from unittest.mock import AsyncMock, MagicMock, patch
    +
    +import pytest
    +
    +from gateway.config import PlatformConfig
    +from gateway.platforms.slack import SlackAdapter
    +
    +
    [redacted:email]
    +def adapter():
    +    config = PlatformConfig(enabled=True, token="[redacted:secret-assignment]")
    +    a = SlackAdapter(config)
    +    a._app = MagicMock()
    +    a._app.client = AsyncMock()
    +    a._bot_user_id = "U_BOT"
    +    a._running = True
    +    a.handle_message = AsyncMock()
    +    return a
    +
    +
    [redacted:email](autouse=True)
    +def _redirect_cache(tmp_path, monkeypatch):
    +    """Point document cache to tmp_path so tests don't touch ~/.hermes."""
    +    monkeypatch.setattr(
    +        "gateway.platforms.base.DOCUMENT_CACHE_DIR", tmp_path / "doc_cache"
    +    )
    +
    +
    +def _channel_event(text: str, ts: str, thread_ts: str = None) -> dict:
    +    """Build a minimal ``message`` event for the Slack Events API
    +    resembling what ``handle_message_event`` would pass through."""
    +    event = {
    +        "channel": "C_CHAN",
    +        "channel_type": "channel",
    +        "user": "U_USER",
    +        "text": text,
    +        "ts": ts,
    +    }
    +    if thread_ts is not None:
    +        event["thread_ts"] = thread_ts
    +    return event
    +
    +
    +class TestChannelSessionScopeDefault:
    +    """``reply_in_thread: true`` is the historical default.  Top-level
    +    channel messages still map ``thread_id = ts`` so each new message
    +    becomes its own threaded session — unchanged from the pre-#15421
    +    behaviour."""
    +
    +    @pytest.mark.asyncio
    +    async def test_top_level_maps_to_ts_when_reply_in_thread_true(self, adapter):
    +        adapter.config.extra["reply_in_thread"] = True
    +        event = _channel_event(
    +            "<@U_BOT> hello",
    +            ts="1700000000.000001",
    +        )
    +
    +        captured = []
    +        adapter.handle_message = AsyncMock(
    +            side_effect=lambda e: captured.append(e)
    +        )
    +        with patch.object(
    +            adapter, "_resolve_user_name",
    +            new=AsyncMock(return_value="testuser"),
    +        ):
    +            await adapter._handle_slack_message(event)
    +
    +        assert len(captured) == 1, (
    +            "handler dropped the top-level channel mention — "
    +            "mention gating misfired"
    +        )
    +        source = captured[0].source
    +        assert source.thread_id == "1700000000.000001", (
    +            "legacy default (reply_in_thread=true) must keep stamping "
    +            "thread_id = ts so each top-level message gets its own "
    +            "threaded session — regression guard"
    +        )
    +
    +    @pytest.mark.asyncio
    +    async def test_top_level_default_behaves_like_true(self, adapter):
    +        """Operators who never set ``reply_in_thread`` must see the
    +        historical behaviour (true).  Pin the default explicitly."""
    +        # Note: no adapter.config.extra["reply_in_thread"] set here.
    +        event = _channel_event(
    +            "<@U_BOT> hello",
    +            ts="1700000000.000002",
    +        )
    +
    +        captured = []
    +        adapter.handle_message = AsyncMock(
    +            side_effect=lambda e: captured.append(e)
    +        )
    +        with patch.object(
    +            adapter, "_resolve_user_name",
    +            new=AsyncMock(return_value="testuser"),
    +        ):
    +            await adapter._handle_slack_message(event)
    +
    +        assert len(captured) == 1
    +        assert captured[0].source.thread_id == "1700000000.000002"
    +
    +
    +class TestChannelSessionScopeShared:
    +    """``reply_in_thread: false`` is the #15421 fix: top-level channel
    +    messages get ``thread_id = None`` so all of them share one
    +    channel-scoped session.  Genuine thread replies still get their
    +    real ``thread_ts``."""
    +
    +    @pytest.mark.asyncio
    +    async def test_top_level_maps_to_none_when_reply_in_thread_false(self, adapter):
    +        adapter.config.extra["reply_in_thread"] = False
    +        event = _channel_event(
    +            "<@U_BOT> hello",
    +            ts="1700000000.000003",
    +        )
    +
    +        captured = []
    +        adapter.handle_message = AsyncMock(
    +            side_effect=lambda e: captured.append(e)
    +        )
    +        with patch.object(
    +            adapter, "_resolve_user_name",
    +            new=AsyncMock(return_value="testuser"),
    +        ):
    +            await adapter._handle_slack_message(event)
    +
    +        assert len(captured) == 1
    +        source = captured[0].source
    +        assert source.thread_id is None, (
    +            "reply_in_thread=false must set thread_id=None for top-level "
    +            "channel messages so the session store groups them under a "
    +            "single channel-scoped session (#15421 bug 1)"
    +        )
    +
    +    @pytest.mark.asyncio
    +    async def test_top_level_reply_to_id_stays_none_when_shared(self, adapter):
    +        """In shared-session mode (``reply_in_thread=false``), top-level
    +        channel messages are normalised to ``thread_ts = None``.  The
    +        outbound check on the ``MessageEvent`` is:
    +
    +            reply_to_message_id = thread_ts if thread_ts != ts else None
    +
    +        With ``thread_ts = None``, ``None != ts`` is True, so the
    +        expression evaluates to ``thread_ts`` itself — which IS
    +        ``None``.  That leaves ``reply_to_message_id`` as ``None`` and
    +        the bot posts a fresh un-threaded channel reply, matching what
    +        ``reply_in_thread=false`` means end-to-end.  This regression
    +        test locks in that invariant (Copilot noted the pre-fix
    +        docstring had the logic reversed).
    +        """
    +        adapter.config.extra["reply_in_thread"] = False
    +        event = _channel_event(
    +            "<@U_BOT> hello",
    +            ts="1700000000.000004",
    +        )
    +
    +        captured = []
    +        adapter.handle_message = AsyncMock(
    +            side_effect=lambda e: captured.append(e)
    +        )
    +        with patch.object(
    +            adapter, "_resolve_user_name",
    +            new=AsyncMock(return_value="testuser"),
    +        ):
    +            await adapter._handle_slack_message(event)
    +
    +        assert captured[0].reply_to_message_id is None, (
    +            "top-level channel messages with reply_in_thread=false "
    +            "must not be threaded (reply_to_message_id=None)"
    +        )
    +
    +    @pytest.mark.asyncio
    +    async def test_thread_reply_scopes_by_thread_even_when_shared(self, adapter):
    +        """Bug 1's fix targets ONLY top-level channel messages.  Genuine
    +        thread replies (``thread_ts != ts``) must still scope per-thread
    +        sessions so multi-person threaded conversations don't collide
    +        with unrelated channel chatter."""
    +        adapter.config.extra["reply_in_thread"] = False
    +        # Reply to an earlier thread root at ts=1700000000.000000
    +        event = _channel_event(
    +            "<@U_BOT> following up",
    +            ts="1700000000.000005",
    +            thread_ts="1700000000.000000",
    +        )
    +
    +        captured = []
    +        adapter.handle_message = AsyncMock(
    +            side_effect=lambda e: captured.append(e)
    +        )
    +        with patch.object(
    +            adapter, "_resolve_user_name",
    +            new=AsyncMock(return_value="testuser"),
    +        ):
    +            await adapter._handle_slack_message(event)
    +
    +        assert len(captured) == 1
    +        source = captured[0].source
    +        assert source.thread_id == "1700000000.000000", (
    +            "genuine thread replies must still scope by thread even "
    +            "when reply_in_thread=false — only TOP-LEVEL messages share "
    +            "the channel-wide session"
    +        )
    +        assert captured[0].reply_to_message_id == "1700000000.000000", (
    +            "reply should thread under the existing thread root"
    +        )
    +
    +
    +class TestThreadReplyAlwaysScopesByThread:
    +    """Cross-cutting invariant: genuine thread replies always scope by
    +    ``thread_ts`` regardless of ``reply_in_thread``.  If this ever
    +    regresses, every thread-scoped conversation leaks across threads."""
    +
    +    @pytest.mark.asyncio
    +    @pytest.mark.parametrize("reply_in_thread", [True, False])
    +    async def test_thread_reply_keyed_by_thread_ts(self, adapter, reply_in_thread):
    +        adapter.config.extra["reply_in_thread"] = reply_in_thread
    +        event = _channel_event(
    +            "<@U_BOT> thread reply",
    +            ts="1700000000.000010",
    +            thread_ts="1700000000.000009",
    +        )
    +
    +        captured = []
    +        adapter.handle_message = AsyncMock(
    +            side_effect=lambda e: captured.append(e)
    +        )
    +        with patch.object(
    +            adapter, "_resolve_user_name",
    +            new=AsyncMock(return_value="testuser"),
    +        ):
    +            await adapter._handle_slack_message(event)
    +
    +        assert len(captured) == 1, (
    +            f"thread reply dropped with reply_in_thread={reply_in_thread}"
    +        )
    +        assert captured[0].source.thread_id == "1700000000.000009"