mirror of
https://github.com/NousResearch/hermes-agent.git
synced 2026-07-23 16:36:23 +00:00
fix(slack): filter unlabeled app and bot authored events
(cherry picked from commit e2e11dab51)
This commit is contained in:
parent
7e42f0143c
commit
c2a49db525
2 changed files with 104 additions and 7 deletions
|
|
@ -2197,6 +2197,12 @@ class SlackAdapter(BasePlatformAdapter):
|
|||
profile = event.get("user_profile")
|
||||
if isinstance(profile, dict) and bool(profile.get("is_bot")):
|
||||
return True
|
||||
# Some Slack app-originated events arrive without subtype=bot_message
|
||||
# or bot_id, but they still carry app_id and no client_msg_id. Real
|
||||
# human-authored messages normally carry client_msg_id, so treat the
|
||||
# combination as app/bot-authored (#35777).
|
||||
if event.get("app_id") and not event.get("client_msg_id"):
|
||||
return True
|
||||
return False
|
||||
|
||||
def _resolve_thread_ts(
|
||||
|
|
@ -3836,11 +3842,29 @@ class SlackAdapter(BasePlatformAdapter):
|
|||
if event_ts and self._dedup.is_duplicate(event_ts):
|
||||
return
|
||||
|
||||
# Bot message filtering (SLACK_ALLOW_BOTS / config allow_bots):
|
||||
# "none" — ignore all bot messages (default, backward-compatible)
|
||||
# "mentions" — accept bot messages only when they @mention us
|
||||
# "all" — accept all bot messages (except our own)
|
||||
if self._event_declares_bot_sender(event):
|
||||
# Bot/app-authored message filtering (SLACK_ALLOW_BOTS / config
|
||||
# allow_bots):
|
||||
# "none" — ignore all bot/app-authored messages (default,
|
||||
# backward-compatible)
|
||||
# "mentions" — accept bot/app-authored messages only when they
|
||||
# @mention us
|
||||
# "all" — accept all bot/app-authored messages (except our own)
|
||||
#
|
||||
# Some Slack app-originated events arrive without subtype=bot_message
|
||||
# or bot_id but still carry app_id and no client_msg_id
|
||||
# (_event_declares_bot_sender covers those markers). Others carry only
|
||||
# a bot *user* id — probe users.info for suspicious unlabeled events:
|
||||
# real human-authored Slack messages normally carry client_msg_id;
|
||||
# bot/app-originated events that slip past the markers often do not.
|
||||
msg_user = event.get("user", "")
|
||||
sender_is_bot = self._event_declares_bot_sender(event)
|
||||
if not sender_is_bot and msg_user and not event.get("client_msg_id"):
|
||||
sender_is_bot = await self._resolve_user_is_bot(
|
||||
msg_user,
|
||||
chat_id=event.get("channel", ""),
|
||||
team_id=str(event.get("team") or event.get("team_id") or ""),
|
||||
)
|
||||
if sender_is_bot:
|
||||
allow_bots = self._slack_allow_bots()
|
||||
if allow_bots == "none":
|
||||
return
|
||||
|
|
@ -3856,7 +3880,6 @@ class SlackAdapter(BasePlatformAdapter):
|
|||
return
|
||||
# "all" falls through to process the message
|
||||
# Always ignore our own messages to prevent echo loops
|
||||
msg_user = event.get("user", "")
|
||||
if msg_user and self._bot_user_id and msg_user == self._bot_user_id:
|
||||
return
|
||||
|
||||
|
|
|
|||
|
|
@ -106,11 +106,20 @@ def _fake_create_task(coro):
|
|||
|
||||
@pytest.fixture()
|
||||
def adapter():
|
||||
config = PlatformConfig(enabled=True, token="xoxb-fake-token")
|
||||
config = PlatformConfig(enabled=True, token="***")
|
||||
a = SlackAdapter(config)
|
||||
# Mock the Slack app client
|
||||
a._app = MagicMock()
|
||||
a._app.client = AsyncMock()
|
||||
a._app.client.users_info = AsyncMock(
|
||||
return_value={
|
||||
"user": {
|
||||
"is_bot": False,
|
||||
"profile": {"display_name": "Test User"},
|
||||
"real_name": "Test User",
|
||||
}
|
||||
}
|
||||
)
|
||||
a._bot_user_id = "U_BOT"
|
||||
a._running = True
|
||||
# Capture events instead of processing them
|
||||
|
|
@ -2426,6 +2435,53 @@ class TestMessageRouting:
|
|||
assert msg_event.text == "please answer exactly BOT_OK"
|
||||
assert msg_event.source.user_name == "AIDx Engineer"
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_app_authored_messages_without_client_msg_id_are_ignored(self, adapter):
|
||||
"""Slack app-authored events can arrive without bot_id/subtype markers."""
|
||||
adapter._app.client.users_info = AsyncMock(
|
||||
return_value={
|
||||
"user": {
|
||||
"is_bot": False,
|
||||
"profile": {"display_name": "helper-app"},
|
||||
"real_name": "Helper App",
|
||||
}
|
||||
}
|
||||
)
|
||||
event = {
|
||||
"text": "workflow reply",
|
||||
"app_id": "A_HELPER",
|
||||
"user": "U_APP_HELPER",
|
||||
"channel": "C123",
|
||||
"channel_type": "im",
|
||||
"ts": "1234567890.000002",
|
||||
}
|
||||
await adapter._handle_slack_message(event)
|
||||
adapter.handle_message.assert_not_called()
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_known_bot_users_ignored_even_without_bot_markers(self, adapter):
|
||||
"""users.info bot identities should still route through bot filtering."""
|
||||
adapter._app.client.users_info = AsyncMock(
|
||||
return_value={
|
||||
"user": {
|
||||
"is_bot": True,
|
||||
"profile": {"display_name": "helper-bot"},
|
||||
"real_name": "Helper Bot",
|
||||
}
|
||||
}
|
||||
)
|
||||
event = {
|
||||
"text": "helper response",
|
||||
"user": "U_HELPER_BOT",
|
||||
"channel": "C123",
|
||||
"channel_type": "im",
|
||||
"ts": "1234567890.000003",
|
||||
}
|
||||
await adapter._handle_slack_message(event)
|
||||
adapter._app.client.users_info.assert_awaited_once_with(user="U_HELPER_BOT")
|
||||
assert adapter._user_is_bot_cache[("", "U_HELPER_BOT")] is True
|
||||
adapter.handle_message.assert_not_called()
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_message_edits_ignored(self, adapter):
|
||||
"""Message edits should be ignored."""
|
||||
|
|
@ -3535,6 +3591,15 @@ class TestThreadReplyHandling:
|
|||
a = SlackAdapter(config)
|
||||
a._app = MagicMock()
|
||||
a._app.client = AsyncMock()
|
||||
a._app.client.users_info = AsyncMock(
|
||||
return_value={
|
||||
"user": {
|
||||
"is_bot": False,
|
||||
"profile": {"display_name": "Test User"},
|
||||
"real_name": "Test User",
|
||||
}
|
||||
}
|
||||
)
|
||||
a._bot_user_id = "U_BOT"
|
||||
a._team_bot_user_ids = {"T_TEAM": "U_BOT"}
|
||||
a._running = True
|
||||
|
|
@ -3919,6 +3984,15 @@ class TestAssistantThreadLifecycle:
|
|||
a = SlackAdapter(config)
|
||||
a._app = MagicMock()
|
||||
a._app.client = AsyncMock()
|
||||
a._app.client.users_info = AsyncMock(
|
||||
return_value={
|
||||
"user": {
|
||||
"is_bot": False,
|
||||
"profile": {"display_name": "Test User"},
|
||||
"real_name": "Test User",
|
||||
}
|
||||
}
|
||||
)
|
||||
a._bot_user_id = "U_BOT"
|
||||
a._team_bot_user_ids = {"T_TEAM": "U_BOT"}
|
||||
a._running = True
|
||||
|
|
|
|||
Loading…
Add table
Add a link
Reference in a new issue