Skip to content

fix(slack): stop injecting thread roots as reply context #21

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
nikitaBarkov merged 1 commit into from
Aug 5, 2026
Merged

fix(slack): stop injecting thread roots as reply context #21

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
46 changes: 11 additions & 35 deletions plugins/platforms/slack/adapter.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -287,7 +287,8 @@ class _ThreadContextCache:
content: str
fetched_at: float = field(default_factory=time.monotonic)
message_count: int = 0
parent_text: str = "" # Raw text of the thread parent (for reply_to_text injection)
# Cached root text used by mention wake checks.
parent_text: str = ""
# The Slack user_id of the thread parent message author. Used by
# _bot_authored_thread_root (#63530) to detect threads whose root was
# posted by the bot via direct chat.postMessage (outside the gateway's
Expand Down Expand Up @@ -4897,10 +4898,8 @@ async def _handle_slack_reaction(self, event: dict, removed: bool = False) -> No
used by the Feishu and Photon adapters — ``reaction:added:<emoji>`` /
``reaction:removed:<emoji>`` — with common Slack reaction names
translated to unicode emoji (👍, 👎, ✅, …) so agents and skills see
the same shape on every platform. Because the synthesized event is
threaded under the reacted-to message, the existing reply-context
plumbing injects the target message's text as ``reply_to_text`` and
the agent sees WHAT was reacted to.
the same shape on every platform. The event is routed to the
reacted-to message's thread, whose history supplies the context.

Message-pipeline routing is OPT-IN via ``slack.reaction_triggers``
(default off) so busy channels don't wake the agent on every emoji.
Expand Down Expand Up @@ -6367,29 +6366,6 @@ async def _handle_slack_message(
None,
)

# Extract reply context if this message is a thread reply.
# Mirrors the Telegram/Discord implementations so that gateway.run
# can inject a `[Replying to: "..."]` prefix when the parent is not
# already in the session history. Uses the thread-context cache when
# available to avoid redundant conversations.replies calls.
reply_to_text = None
if thread_ts and thread_ts != ts:
try:
reply_to_text = (
await self._fetch_thread_parent_text(
channel_id=channel_id,
thread_ts=thread_ts,
team_id=team_id,
)
or None
)
if reply_to_text:
reply_to_text = await self._humanize_user_mentions(
reply_to_text, chat_id=channel_id, team_id=team_id
)
except Exception: # pragma: no cover - defensive
reply_to_text = None

# Humanize remaining user mentions: the bot's own mention was already
# stripped above, so any ``<@UID>`` left in the trigger text refers to
# OTHER participants. Render them as ``@DisplayName`` so the agent can
Expand All @@ -6412,7 +6388,9 @@ async def _handle_slack_message(
reply_to_message_id=thread_ts if thread_ts != ts else None,
channel_prompt=_channel_prompt,
channel_context=channel_context,
reply_to_text=reply_to_text,
# thread_ts identifies the thread root, not an explicit reply;
# channel_context hydrates the root separately.
reply_to_text=None,
auto_skill=_auto_skill,
metadata={
"slack_team_id": team_id,
Expand Down Expand Up @@ -7463,8 +7441,7 @@ async def _format_thread_context(

When ``after_ts`` is set, only messages with ts strictly greater than
the watermark are included (delta refresh, #23918); the thread parent
text is still captured regardless so reply_to_text callers keep
working from the shared cache.
text is still captured in the shared cache.

Returns ``(content, parent_text)``.
"""
Expand Down Expand Up @@ -7613,16 +7590,15 @@ async def _fetch_thread_parent_text(
) -> str:
"""Return the text of the thread parent message.

Used for reply_to_text injection (mention stripped) and for the
parent-mentioned-bot wake check (#24848 — pass
``strip_bot_mention=False`` so the ``<@bot>`` token is preserved).
Used to check whether the root mentions the bot (#24848). Set
``strip_bot_mention=False`` to preserve the mention.

Uses the same per-thread cache as :meth:`_fetch_thread_context` to avoid
hitting ``conversations.replies`` twice. Falls back to a cheap single-
message fetch (``limit=1, inclusive=True``) when the cache is cold.

Returns empty string on any failure — callers should treat an empty
return as "no parent context to inject".
return as an unavailable parent message.
"""
cache_key = f"{channel_id}:{thread_ts}:{team_id}"
now = time.monotonic()
Expand Down
59 changes: 47 additions & 12 deletions tests/gateway/test_slack.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,7 @@
import pytest

import agent.secret_scope as secret_scope
from gateway.config import Platform, PlatformConfig
from gateway.config import GatewayConfig, Platform, PlatformConfig
from gateway.run import GatewayRunner
from gateway.platforms.base import (
MessageEvent,
Expand Down Expand Up @@ -3711,15 +3711,11 @@ async def test_dm_toplevel_progress_uses_message_ts_as_thread(self, adapter):
)


class TestSlackReplyToText:
"""Ensure MessageEvent.reply_to_text is populated on thread replies so
gateway.run can inject a ``[Replying to: "..."]`` prefix (parity with
Telegram/Discord/Feishu/WeCom)."""
class TestSlackThreadParentContext:
"""Ensure Slack thread roots are hydrated once, not injected every turn."""

@pytest.mark.asyncio
async def test_slack_reply_to_text_set_on_thread_reply(self, adapter):
"""When a thread reply arrives and the parent was posted by a bot
(e.g. cron summary), reply_to_text must carry the parent's text."""
async def test_thread_root_uses_channel_context_not_reply_to_text(self, adapter):
adapter._channel_team = {} # primary workspace only
adapter._team_bot_user_ids = {}

Expand Down Expand Up @@ -3757,10 +3753,49 @@ async def test_slack_reply_to_text_set_on_thread_reply(self, adapter):
), "handle_message must be invoked for thread-reply DM"
msg_event = adapter.handle_message.call_args[0][0]
assert msg_event.reply_to_message_id == "1000.0"
# The critical assertion: parent text is exposed as reply_to_text so the
# gateway can inject it when not already in the session history.
assert msg_event.reply_to_text is not None
assert "メール要約" in msg_event.reply_to_text
assert "メール要約" in msg_event.channel_context
assert msg_event.reply_to_text is None

@pytest.mark.asyncio
async def test_active_thread_does_not_refetch_root_as_reply_text(self, adapter):
adapter._has_active_session_for_thread = MagicMock(return_value=True)
adapter._fetch_thread_parent_text = AsyncMock(return_value="original task")

event = {
"text": "one more detail",
"user": "U_USER",
"channel": "D123",
"channel_type": "im",
"ts": "1001.0",
"thread_ts": "1000.0",
}

with patch.object(
adapter, "_resolve_user_name", new=AsyncMock(return_value="Alice")
):
await adapter._handle_slack_message(event)

msg_event = adapter.handle_message.call_args[0][0]
assert msg_event.channel_context is None
assert msg_event.reply_to_message_id == "1000.0"
assert msg_event.reply_to_text is None
adapter._fetch_thread_parent_text.assert_not_awaited()

runner = object.__new__(GatewayRunner)
runner.config = GatewayConfig(
platforms={
Platform.SLACK: PlatformConfig(enabled=True, token="fake")
}
)
runner.adapters = {}
prepared = await runner._prepare_inbound_message_text(
event=msg_event,
source=msg_event.source,
history=[{"role": "user", "content": "original task"}],
)

assert prepared == "one more detail"
assert "[Replying to:" not in prepared


# ---------------------------------------------------------------------------
Expand Down
Loading