mirror of
https://github.com/NousResearch/hermes-agent.git
synced 2026-06-09 08:21:50 +00:00
feat(gateway): Enable Slack thread replies without explicit @mentions
When a user replies in a Slack thread where the bot has an active conversation session, the bot now processes the message even without an explicit @mention. This improves UX for ongoing threaded discussions. Changes: - Added set_session_store() to BasePlatformAdapter for adapters to check active sessions - Modified SlackAdapter to detect thread replies and check if a session exists for that thread before requiring @mentions - Updated GatewayRunner to inject the session store into adapters - Added comprehensive tests for the new behavior Fixes: Thread replies without @jarvis are now processed if there is an active session, matching user expectations for conversation flow
This commit is contained in:
eizus
Teknium
parent
9b6e5f6a04
commit
4ec615b0c2
4 changed files with 235 additions and 3 deletions
|
|
@ -569,6 +569,16 @@ class BasePlatformAdapter(ABC):
|
|||
"""
|
||||
self._message_handler = handler
|
||||
|
||||
def set_session_store(self, session_store: Any) -> None:
|
||||
"""
|
||||
Set the session store for checking active sessions.
|
||||
|
||||
Used by adapters that need to check if a thread/conversation
|
||||
has an active session before processing messages (e.g., Slack
|
||||
thread replies without explicit mentions).
|
||||
"""
|
||||
self._session_store = session_store
|
||||
|
||||
@abstractmethod
|
||||
async def connect(self) -> bool:
|
||||
"""
|
||||
|
|
|
|||
|
|
@ -766,11 +766,28 @@ class SlackAdapter(BasePlatformAdapter):
|
|||
else:
|
||||
thread_ts = event.get("thread_ts") or ts # ts fallback for channels
|
||||
|
||||
# In channels, only respond if bot is mentioned
|
||||
# In channels, only respond if bot is mentioned OR if this is a
|
||||
# reply in a thread where the bot has an active session.
|
||||
bot_uid = self._team_bot_user_ids.get(team_id, self._bot_user_id)
|
||||
if not is_dm and bot_uid:
|
||||
if f"<@{bot_uid}>" not in text:
|
||||
is_mentioned = bot_uid and f"<@{bot_uid}>" in text
|
||||
|
||||
if not is_dm and bot_uid and not is_mentioned:
|
||||
# Check if this is a thread reply (thread_ts exists and differs from ts)
|
||||
event_thread_ts = event.get("thread_ts")
|
||||
is_thread_reply = event_thread_ts and event_thread_ts != ts
|
||||
|
||||
if is_thread_reply and self._has_active_session_for_thread(
|
||||
channel_id=channel_id,
|
||||
thread_ts=event_thread_ts,
|
||||
user_id=user_id,
|
||||
):
|
||||
# Allow thread replies without mention if there's an active session
|
||||
pass
|
||||
else:
|
||||
# Not a thread reply or no active session - ignore
|
||||
return
|
||||
|
||||
if is_mentioned:
|
||||
# Strip the bot mention from the text
|
||||
text = text.replace(f"<@{bot_uid}>", "").strip()
|
||||
|
||||
|
|
@ -936,6 +953,68 @@ class SlackAdapter(BasePlatformAdapter):
|
|||
|
||||
await self.handle_message(event)
|
||||
|
||||
def _has_active_session_for_thread(
|
||||
self,
|
||||
channel_id: str,
|
||||
thread_ts: str,
|
||||
user_id: str,
|
||||
) -> bool:
|
||||
"""Check if there's an active session for a thread.
|
||||
|
||||
Used to determine if thread replies without @mentions should be
|
||||
processed (they should if there's an active session).
|
||||
|
||||
Args:
|
||||
channel_id: The Slack channel ID
|
||||
thread_ts: The thread timestamp (parent message ts)
|
||||
user_id: The user ID of the sender
|
||||
|
||||
Returns:
|
||||
True if there's an active session for this thread
|
||||
"""
|
||||
session_store = getattr(self, "_session_store", None)
|
||||
if not session_store:
|
||||
return False
|
||||
|
||||
try:
|
||||
# Build a SessionSource for this thread
|
||||
from gateway.session import SessionSource
|
||||
from gateway.config import Platform
|
||||
|
||||
source = SessionSource(
|
||||
platform=Platform.SLACK,
|
||||
chat_id=channel_id,
|
||||
chat_type="group",
|
||||
user_id=user_id,
|
||||
thread_id=thread_ts,
|
||||
)
|
||||
|
||||
# Generate the session key using the same logic as SessionStore
|
||||
# This mirrors the logic in build_session_key for group sessions
|
||||
key_parts = ["agent:main", "slack", "group", channel_id, thread_ts]
|
||||
|
||||
# Include user_id if group_sessions_per_user is enabled
|
||||
# We check the session store config if available
|
||||
group_sessions_per_user = getattr(
|
||||
session_store, "config", {}
|
||||
)
|
||||
if hasattr(group_sessions_per_user, "group_sessions_per_user"):
|
||||
group_sessions_per_user = group_sessions_per_user.group_sessions_per_user
|
||||
else:
|
||||
group_sessions_per_user = True # Default
|
||||
|
||||
if group_sessions_per_user and user_id:
|
||||
key_parts.append(str(user_id))
|
||||
|
||||
session_key = ":".join(key_parts)
|
||||
|
||||
# Check if the session exists in the store
|
||||
session_store._ensure_loaded()
|
||||
return session_key in session_store._entries
|
||||
except Exception:
|
||||
# If anything goes wrong, default to False (require mention)
|
||||
return False
|
||||
|
||||
async def _download_slack_file(self, url: str, ext: str, audio: bool = False, team_id: str = "") -> str:
|
||||
"""Download a Slack file using the bot token for auth, with retry."""
|
||||
import asyncio
|
||||
|
|
|
|||
|
|
@ -1127,6 +1127,7 @@ class GatewayRunner:
|
|||
# Set up message + fatal error handlers
|
||||
adapter.set_message_handler(self._handle_message)
|
||||
adapter.set_fatal_error_handler(self._handle_adapter_fatal_error)
|
||||
adapter.set_session_store(self.session_store)
|
||||
|
||||
# Try to connect
|
||||
logger.info("Connecting to %s...", platform.value)
|
||||
|
|
@ -1424,6 +1425,7 @@ class GatewayRunner:
|
|||
|
||||
adapter.set_message_handler(self._handle_message)
|
||||
adapter.set_fatal_error_handler(self._handle_adapter_fatal_error)
|
||||
adapter.set_session_store(self.session_store)
|
||||
|
||||
success = await adapter.connect()
|
||||
if success:
|
||||
|
|
|
|||
|
|
@ -699,6 +699,147 @@ class TestReactions:
|
|||
assert remove_calls[0].kwargs["name"] == "eyes"
|
||||
|
||||
|
||||
# ---------------------------------------------------------------------------
|
||||
# TestThreadReplyHandling
|
||||
# ---------------------------------------------------------------------------
|
||||
|
||||
|
||||
class TestThreadReplyHandling:
|
||||
"""Test thread reply processing without explicit bot mentions."""
|
||||
|
||||
@pytest.fixture()
|
||||
def mock_session_store(self):
|
||||
"""Create a mock session store with entries dict."""
|
||||
store = MagicMock()
|
||||
store._entries = {}
|
||||
store._ensure_loaded = MagicMock()
|
||||
store.config = MagicMock()
|
||||
store.config.group_sessions_per_user = True
|
||||
return store
|
||||
|
||||
@pytest.fixture()
|
||||
def adapter_with_session_store(self, mock_session_store):
|
||||
"""Create an adapter with a mock session store attached."""
|
||||
config = PlatformConfig(enabled=True, token="***")
|
||||
a = SlackAdapter(config)
|
||||
a._app = MagicMock()
|
||||
a._app.client = AsyncMock()
|
||||
a._bot_user_id = "U_BOT"
|
||||
a._team_bot_user_ids = {"T_TEAM": "U_BOT"}
|
||||
a._running = True
|
||||
a.handle_message = AsyncMock()
|
||||
a.set_session_store(mock_session_store)
|
||||
return a
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_thread_reply_without_mention_no_session_ignored(
|
||||
self, adapter_with_session_store, mock_session_store
|
||||
):
|
||||
"""Thread replies without mention should be ignored if no active session."""
|
||||
mock_session_store._entries = {} # No active sessions
|
||||
|
||||
event = {
|
||||
"text": "Just replying in the thread",
|
||||
"user": "U_USER",
|
||||
"channel": "C123",
|
||||
"ts": "123.456",
|
||||
"thread_ts": "123.000", # Different from ts - this is a reply
|
||||
"channel_type": "channel",
|
||||
"team": "T_TEAM",
|
||||
}
|
||||
await adapter_with_session_store._handle_slack_message(event)
|
||||
adapter_with_session_store.handle_message.assert_not_called()
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_thread_reply_without_mention_with_session_processed(
|
||||
self, adapter_with_session_store, mock_session_store
|
||||
):
|
||||
"""Thread replies without mention should be processed if there's an active session."""
|
||||
# Simulate an active session for this thread
|
||||
session_key = "agent:main:slack:group:C123:123.000:U_USER"
|
||||
mock_session_store._entries = {session_key: MagicMock()}
|
||||
|
||||
event = {
|
||||
"text": "Follow-up question",
|
||||
"user": "U_USER",
|
||||
"channel": "C123",
|
||||
"ts": "123.456",
|
||||
"thread_ts": "123.000", # Reply in thread 123.000
|
||||
"channel_type": "channel",
|
||||
"team": "T_TEAM",
|
||||
}
|
||||
await adapter_with_session_store._handle_slack_message(event)
|
||||
adapter_with_session_store.handle_message.assert_called_once()
|
||||
|
||||
# Verify the text is passed through unchanged (no mention stripping needed)
|
||||
msg_event = adapter_with_session_store.handle_message.call_args[0][0]
|
||||
assert msg_event.text == "Follow-up question"
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_thread_reply_with_mention_strips_bot_id(
|
||||
self, adapter_with_session_store, mock_session_store
|
||||
):
|
||||
"""Thread replies with @mention should still strip the bot ID."""
|
||||
# Even with a session, mentions should be stripped
|
||||
session_key = "agent:main:slack:group:C123:123.000:U_USER"
|
||||
mock_session_store._entries = {session_key: MagicMock()}
|
||||
|
||||
event = {
|
||||
"text": "<@U_BOT> thanks for the help",
|
||||
"user": "U_USER",
|
||||
"channel": "C123",
|
||||
"ts": "123.456",
|
||||
"thread_ts": "123.000",
|
||||
"channel_type": "channel",
|
||||
"team": "T_TEAM",
|
||||
}
|
||||
await adapter_with_session_store._handle_slack_message(event)
|
||||
adapter_with_session_store.handle_message.assert_called_once()
|
||||
|
||||
msg_event = adapter_with_session_store.handle_message.call_args[0][0]
|
||||
assert "<@U_BOT>" not in msg_event.text
|
||||
assert msg_event.text == "thanks for the help"
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_top_level_message_requires_mention_even_with_session(
|
||||
self, adapter_with_session_store, mock_session_store
|
||||
):
|
||||
"""Top-level channel messages should require mention even if session exists."""
|
||||
# Session exists but this is a top-level message (no thread_ts)
|
||||
session_key = "agent:main:slack:group:C123:123.000:U_USER"
|
||||
mock_session_store._entries = {session_key: MagicMock()}
|
||||
|
||||
event = {
|
||||
"text": "New question without mention",
|
||||
"user": "U_USER",
|
||||
"channel": "C123",
|
||||
"ts": "456.789",
|
||||
# No thread_ts - this is a top-level message
|
||||
"channel_type": "channel",
|
||||
"team": "T_TEAM",
|
||||
}
|
||||
await adapter_with_session_store._handle_slack_message(event)
|
||||
adapter_with_session_store.handle_message.assert_not_called()
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_no_session_store_ignores_thread_replies(
|
||||
self, adapter
|
||||
):
|
||||
"""If no session store is attached, thread replies without mention should be ignored."""
|
||||
# adapter fixture has no session store attached
|
||||
event = {
|
||||
"text": "Thread reply without mention",
|
||||
"user": "U_USER",
|
||||
"channel": "C123",
|
||||
"ts": "123.456",
|
||||
"thread_ts": "123.000",
|
||||
"channel_type": "channel",
|
||||
"team": "T_TEAM",
|
||||
}
|
||||
await adapter._handle_slack_message(event)
|
||||
adapter.handle_message.assert_not_called()
|
||||
|
||||
|
||||
# ---------------------------------------------------------------------------
|
||||
# TestUserNameResolution
|
||||
# ---------------------------------------------------------------------------
|
||||
|
|
|
|||
Loading…
Add table
Add a link
Reference in a new issue