fix(slack): gate inbound turns on a conversational-subtype allowlist
Housekeeping subtypes (channel_join/leave/topic/name/purpose, convert_to_private/public, pins, deletions) are not a person speaking, yet _prefilter_inbound only rejected message_changed/message_deleted, so each of them started a full agent turn in free-response channels. Replace the denylist with an allowlist: a message passes when subtype is absent, file_share, thread_broadcast or me_message; everything else is dropped. Fixes #110778.
This commit is contained in:
@@ -4301,8 +4301,21 @@ class SlackAdapter(BasePlatformAdapter):
|
||||
return None
|
||||
if await self._drop_bot_sender(event):
|
||||
return None
|
||||
# Edits were normalized above so an @mention added by edit can wake the bot once.
|
||||
if event.get("subtype") == "message_deleted":
|
||||
# Edits were normalized above so an @mention added by edit can wake the bot once,
|
||||
# which also means their subtype is gone by the time this check runs.
|
||||
# Housekeeping subtypes (joins/leaves, topic/name/purpose changes, convert_to_private/
|
||||
# public, pins, deletions, file comments...) are not a person speaking, so they must
|
||||
# not start a turn in free-response channels (#110778). Allowlist rather than denylist
|
||||
# so subtypes Slack adds later are dropped instead of silently readmitted.
|
||||
# ``file_share`` passes: a human attaching a file is a person speaking, and the
|
||||
# ``file_shared`` fallback synthesizes exactly this subtype. ``thread_broadcast``
|
||||
# passes: a human sharing a threaded reply into the channel carries user/text.
|
||||
# ``me_message`` passes: ``/me`` is a person speaking.
|
||||
subtype = event.get("subtype")
|
||||
if subtype not in (None, "", "file_share", "thread_broadcast", "me_message"):
|
||||
logger.debug(
|
||||
"[Slack] Dropping non-conversational message subtype=%s in channel %s",
|
||||
subtype, channel_id)
|
||||
return None
|
||||
return event, dedup_team_id, channel_id
|
||||
|
||||
|
||||
@@ -6049,3 +6049,76 @@ class TestAgentSessionsApiRouting:
|
||||
thread_ts="171234.0001",
|
||||
title="Summarize the incident",
|
||||
)
|
||||
|
||||
|
||||
# ---------------------------------------------------------------------------
|
||||
# TestNonConversationalSubtypeAllowlist
|
||||
# ---------------------------------------------------------------------------
|
||||
|
||||
|
||||
class TestNonConversationalSubtypeAllowlist:
|
||||
"""#110778 — housekeeping subtypes must not start a turn in free-response
|
||||
channels. The gate is an allowlist so subtypes Slack adds later are dropped
|
||||
instead of silently readmitted."""
|
||||
|
||||
@staticmethod
|
||||
def _event(subtype):
|
||||
event = {
|
||||
"type": "message",
|
||||
"user": "U_HUMAN",
|
||||
"text": "hello",
|
||||
"ts": "12345.6789",
|
||||
"channel": "C_FREE",
|
||||
}
|
||||
if subtype is not None:
|
||||
event["subtype"] = subtype
|
||||
return event
|
||||
|
||||
@pytest.mark.asyncio
|
||||
@pytest.mark.parametrize(
|
||||
"subtype",
|
||||
[
|
||||
"channel_join",
|
||||
"channel_leave",
|
||||
"channel_topic",
|
||||
"channel_purpose",
|
||||
"channel_name",
|
||||
"channel_convert_to_private",
|
||||
"channel_convert_to_public",
|
||||
"pinned_item",
|
||||
"unpinned_item",
|
||||
"message_deleted",
|
||||
"file_comment",
|
||||
],
|
||||
)
|
||||
async def test_housekeeping_subtypes_are_dropped(self, adapter, subtype):
|
||||
assert await adapter._prefilter_inbound(self._event(subtype), None) is None
|
||||
adapter.handle_message.assert_not_awaited()
|
||||
|
||||
@pytest.mark.asyncio
|
||||
@pytest.mark.parametrize(
|
||||
"subtype", [None, "file_share", "thread_broadcast", "me_message"]
|
||||
)
|
||||
async def test_conversational_subtypes_pass(self, adapter, subtype):
|
||||
accepted = await adapter._prefilter_inbound(self._event(subtype), None)
|
||||
assert accepted is not None
|
||||
assert accepted[0].get("channel") == "C_FREE"
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_edited_message_still_wakes_the_bot(self, adapter):
|
||||
event = {
|
||||
"type": "message",
|
||||
"subtype": "message_changed",
|
||||
"channel": "C_FREE",
|
||||
"ts": "12340.0000",
|
||||
"message": {
|
||||
"type": "message",
|
||||
"user": "U_HUMAN",
|
||||
"text": "edited hello",
|
||||
"ts": "12345.6789",
|
||||
"edited": {"ts": "12346.0000"},
|
||||
},
|
||||
}
|
||||
accepted = await adapter._prefilter_inbound(event, None)
|
||||
assert accepted is not None
|
||||
assert accepted[0].get("text") == "edited hello"
|
||||
|
||||
Reference in New Issue
Block a user