fix(slack): evict oldest tracked thread timestamps

This commit is contained in:
markoub 2026-06-23 01:25:34 +02:00 committed by Teknium
parent d8cd7ac2e7
commit 4fb1796331
2 changed files with 74 additions and 35 deletions

View file

@ -742,11 +742,11 @@ class SlackAdapter(BasePlatformAdapter):
self._clarify_resolved: Dict[str, bool] = {}
# Track timestamps of messages sent by the bot so we can respond
# to thread replies even without an explicit @mention.
self._bot_message_ts: set = set()
self._bot_message_ts: set[str] = set()
self._BOT_TS_MAX = 5000 # cap to avoid unbounded growth
# Track threads where the bot has been @mentioned — once mentioned,
# respond to ALL subsequent messages in that thread automatically.
self._mentioned_threads: set = set()
self._mentioned_threads: set[str] = set()
self._MENTIONED_THREADS_MAX = 5000
# Assistant thread metadata keyed by (team_id, channel_id, thread_ts).
# Slack's AI Assistant lifecycle events can arrive before/alongside
@ -827,6 +827,43 @@ class SlackAdapter(BasePlatformAdapter):
await result
break
@staticmethod
def _slack_timestamp_sort_key(ts: str) -> Tuple[int, int, str]:
"""Return a chronological, deterministic sort key for Slack timestamps."""
seconds, _, fraction = str(ts).partition(".")
try:
seconds_int = int(seconds)
except ValueError:
seconds_int = 0
try:
fraction_int = int((fraction + "000000")[:6] or "0")
except ValueError:
fraction_int = 0
return seconds_int, fraction_int, str(ts)
@classmethod
def _discard_oldest_slack_timestamps(
cls, timestamps: set[str], count: int
) -> None:
"""Discard the oldest Slack timestamps from a bounded tracking set."""
if count <= 0:
return
for old_ts in sorted(timestamps, key=cls._slack_timestamp_sort_key)[:count]:
timestamps.discard(old_ts)
def _trim_bot_message_timestamps(self) -> None:
if len(self._bot_message_ts) <= self._BOT_TS_MAX:
return
excess = len(self._bot_message_ts) - self._BOT_TS_MAX // 2
self._discard_oldest_slack_timestamps(self._bot_message_ts, excess)
def _trim_mentioned_threads(self) -> None:
if len(self._mentioned_threads) <= self._MENTIONED_THREADS_MAX:
return
self._discard_oldest_slack_timestamps(
self._mentioned_threads, self._MENTIONED_THREADS_MAX // 2
)
def _start_socket_mode_handler(self) -> None:
"""Start the Slack Socket Mode background task."""
if not self._app or not self._app_token:
@ -2002,10 +2039,7 @@ class SlackAdapter(BasePlatformAdapter):
# Also register the thread root so replies-to-my-replies work
if thread_ts:
self._bot_message_ts.add(thread_ts)
if len(self._bot_message_ts) > self._BOT_TS_MAX:
excess = len(self._bot_message_ts) - self._BOT_TS_MAX // 2
for old_ts in list(self._bot_message_ts)[:excess]:
self._bot_message_ts.discard(old_ts)
self._trim_bot_message_timestamps()
return SendResult(
success=True,
@ -2545,10 +2579,7 @@ class SlackAdapter(BasePlatformAdapter):
if not thread_ts:
return
self._bot_message_ts.add(thread_ts)
if len(self._bot_message_ts) > self._BOT_TS_MAX:
excess = len(self._bot_message_ts) - self._BOT_TS_MAX // 2
for old_ts in list(self._bot_message_ts)[:excess]:
self._bot_message_ts.discard(old_ts)
self._trim_bot_message_timestamps()
def _is_retryable_upload_error(self, exc: Exception) -> bool:
"""Best-effort detection for transient Slack upload failures."""
@ -3772,12 +3803,7 @@ class SlackAdapter(BasePlatformAdapter):
if not thread_ts:
return
self._mentioned_threads.add(thread_ts)
if len(self._mentioned_threads) > self._MENTIONED_THREADS_MAX:
to_remove = list(self._mentioned_threads)[
: self._MENTIONED_THREADS_MAX // 2
]
for t in to_remove:
self._mentioned_threads.discard(t)
self._trim_mentioned_threads()
async def _bot_authored_thread_root(
self, channel_id: str, thread_ts: str, team_id: str = ""

View file

@ -1109,18 +1109,21 @@ class TestThreadEngagement:
# Thread root should also be tracked
assert "8000.0" in adapter._bot_message_ts
@pytest.mark.asyncio
async def test_bot_message_ts_cap(self):
"""Verify memory is bounded when many messages are sent."""
def test_bot_message_ts_cap_evicts_oldest_timestamps(self):
"""Bot thread tracking evicts the oldest Slack timestamps first."""
adapter = _make_adapter()
adapter._BOT_TS_MAX = 10 # low cap for testing
mock_client = adapter._team_clients["T1"]
adapter._BOT_TS_MAX = 4
for i in range(20):
mock_client.chat_postMessage = AsyncMock(return_value={"ts": f"{i}.0"})
await adapter.send(chat_id="C1", content=f"msg {i}")
for ts in [
"1000.000002",
"999.999999",
"1000.000004",
"1000.000001",
"1000.000003",
]:
adapter._record_uploaded_file_thread("C1", ts)
assert len(adapter._bot_message_ts) <= 10
assert adapter._bot_message_ts == {"1000.000003", "1000.000004"}
def test_mentioned_threads_populated_on_mention(self):
"""When bot is @mentioned in a thread, that thread is tracked."""
@ -1129,14 +1132,24 @@ class TestThreadEngagement:
adapter._mentioned_threads.add("1000.0")
assert "1000.0" in adapter._mentioned_threads
def test_mentioned_threads_cap(self):
"""Verify _mentioned_threads is bounded."""
def test_mentioned_threads_cap_evicts_oldest_timestamps(self):
"""Mentioned-thread tracking evicts the oldest Slack timestamps first."""
adapter = _make_adapter()
adapter._MENTIONED_THREADS_MAX = 10
for i in range(15):
adapter._mentioned_threads.add(f"{i}.0")
if len(adapter._mentioned_threads) > adapter._MENTIONED_THREADS_MAX:
to_remove = list(adapter._mentioned_threads)[:adapter._MENTIONED_THREADS_MAX // 2]
for t in to_remove:
adapter._mentioned_threads.discard(t)
assert len(adapter._mentioned_threads) <= 10
adapter._MENTIONED_THREADS_MAX = 4
adapter._mentioned_threads.update(
{
"1000.000002",
"999.999999",
"1000.000004",
"1000.000001",
"1000.000003",
}
)
adapter._trim_mentioned_threads()
assert adapter._mentioned_threads == {
"1000.000002",
"1000.000003",
"1000.000004",
}