fix(slack): per-thread sessions for DMs by default
Each top-level Slack DM now gets its own Hermes session, matching the per-thread behavior channels already have. Previously all top-level DM messages shared one continuous session because thread_ts was None, causing context to accumulate across unrelated conversations. The behavior is controlled by platforms.slack.extra.dm_top_level_threads_as_sessions in config.yaml (default: true). Set to false to restore legacy behavior. Based on PR #10789 by helix4u. Changes from original: - Default flipped to true (was opt-in, now opt-out) - Removed env var fallback (config.yaml only per project policy) - Tests updated to cover both default and opt-out paths
This commit is contained in:
parent
daef0519e9
commit
bd7e272c1f
@ -366,6 +366,20 @@ class SlackAdapter(BasePlatformAdapter):
|
|||||||
# in an assistant-enabled context. Falls back to reactions.
|
# in an assistant-enabled context. Falls back to reactions.
|
||||||
logger.debug("[Slack] assistant.threads.setStatus failed: %s", e)
|
logger.debug("[Slack] assistant.threads.setStatus failed: %s", e)
|
||||||
|
|
||||||
|
def _dm_top_level_threads_as_sessions(self) -> bool:
|
||||||
|
"""Whether top-level Slack DMs get per-message session threads.
|
||||||
|
|
||||||
|
Defaults to ``True`` so each visible DM reply thread is isolated as its
|
||||||
|
own Hermes session — matching the per-thread behavior channels already
|
||||||
|
have. Set ``platforms.slack.extra.dm_top_level_threads_as_sessions``
|
||||||
|
to ``false`` in config.yaml to revert to the legacy behavior where all
|
||||||
|
top-level DMs share one continuous session.
|
||||||
|
"""
|
||||||
|
raw = self.config.extra.get("dm_top_level_threads_as_sessions")
|
||||||
|
if raw is None:
|
||||||
|
return True # default: each DM thread is its own session
|
||||||
|
return str(raw).strip().lower() in ("1", "true", "yes", "on")
|
||||||
|
|
||||||
def _resolve_thread_ts(
|
def _resolve_thread_ts(
|
||||||
self,
|
self,
|
||||||
reply_to: Optional[str] = None,
|
reply_to: Optional[str] = None,
|
||||||
@ -996,10 +1010,14 @@ class SlackAdapter(BasePlatformAdapter):
|
|||||||
# Build thread_ts for session keying.
|
# Build thread_ts for session keying.
|
||||||
# In channels: fall back to ts so each top-level @mention starts a
|
# In channels: fall back to ts so each top-level @mention starts a
|
||||||
# new thread/session (the bot always replies in a thread).
|
# new thread/session (the bot always replies in a thread).
|
||||||
# In DMs: only use the real thread_ts — top-level DMs should share
|
# In DMs: fall back to ts so each top-level DM reply thread gets
|
||||||
# one continuous session, threaded DMs get their own session.
|
# its own session key (matching channel behavior). Set
|
||||||
|
# dm_top_level_threads_as_sessions: false in config to revert to
|
||||||
|
# legacy single-session-per-DM-channel behavior.
|
||||||
if is_dm:
|
if is_dm:
|
||||||
thread_ts = event.get("thread_ts") or assistant_meta.get("thread_ts") # None for top-level DMs
|
thread_ts = event.get("thread_ts") or assistant_meta.get("thread_ts")
|
||||||
|
if not thread_ts and self._dm_top_level_threads_as_sessions():
|
||||||
|
thread_ts = ts
|
||||||
else:
|
else:
|
||||||
thread_ts = event.get("thread_ts") or ts # ts fallback for channels
|
thread_ts = event.get("thread_ts") or ts # ts fallback for channels
|
||||||
|
|
||||||
|
|||||||
@ -1678,11 +1678,11 @@ class TestProgressMessageThread:
|
|||||||
msg_event = captured_events[0]
|
msg_event = captured_events[0]
|
||||||
source = msg_event.source
|
source = msg_event.source
|
||||||
|
|
||||||
# For a top-level DM: source.thread_id should remain None
|
# With default dm_top_level_threads_as_sessions=True, source.thread_id
|
||||||
# (session keying must not be affected)
|
# should equal the message ts so each DM thread gets its own session.
|
||||||
assert source.thread_id is None, (
|
assert source.thread_id == "1234567890.000001", (
|
||||||
"source.thread_id must stay None for top-level DMs "
|
"source.thread_id must equal the message ts for top-level DMs "
|
||||||
"so they share one continuous session"
|
"so each reply thread gets its own session"
|
||||||
)
|
)
|
||||||
|
|
||||||
# The message_id should be the event's ts — this is what the gateway
|
# The message_id should be the event's ts — this is what the gateway
|
||||||
@ -1707,6 +1707,34 @@ class TestProgressMessageThread:
|
|||||||
"ensuring progress messages land in the thread"
|
"ensuring progress messages land in the thread"
|
||||||
)
|
)
|
||||||
|
|
||||||
|
@pytest.mark.asyncio
|
||||||
|
async def test_dm_toplevel_shares_session_when_disabled(self, adapter):
|
||||||
|
"""Opting out restores legacy single-session-per-DM-channel behavior."""
|
||||||
|
adapter.config.extra["dm_top_level_threads_as_sessions"] = False
|
||||||
|
|
||||||
|
event = {
|
||||||
|
"channel": "D_DM",
|
||||||
|
"channel_type": "im",
|
||||||
|
"user": "U_USER",
|
||||||
|
"text": "Hello bot",
|
||||||
|
"ts": "1234567890.000001",
|
||||||
|
}
|
||||||
|
|
||||||
|
captured_events = []
|
||||||
|
adapter.handle_message = AsyncMock(side_effect=lambda e: captured_events.append(e))
|
||||||
|
|
||||||
|
with patch.object(adapter, "_resolve_user_name", new=AsyncMock(return_value="testuser")):
|
||||||
|
await adapter._handle_slack_message(event)
|
||||||
|
|
||||||
|
assert len(captured_events) == 1
|
||||||
|
msg_event = captured_events[0]
|
||||||
|
source = msg_event.source
|
||||||
|
|
||||||
|
assert source.thread_id is None, (
|
||||||
|
"source.thread_id must stay None when "
|
||||||
|
"dm_top_level_threads_as_sessions is disabled"
|
||||||
|
)
|
||||||
|
|
||||||
@pytest.mark.asyncio
|
@pytest.mark.asyncio
|
||||||
async def test_channel_mention_progress_uses_thread_ts(self, adapter):
|
async def test_channel_mention_progress_uses_thread_ts(self, adapter):
|
||||||
"""Progress messages for a channel @mention should go into the reply thread."""
|
"""Progress messages for a channel @mention should go into the reply thread."""
|
||||||
|
|||||||
Loading…
x
Reference in New Issue
Block a user