From c17086b3505ca66d319ccc3f624f6ccc6151b2e2 Mon Sep 17 00:00:00 2001 From: Rezha Julio Date: Fri, 3 Jul 2026 14:35:10 +0700 Subject: [PATCH 1/3] fix(anti-spam): delete all duplicate messages, not just triggering one handle_duplicate_spam only called update.message.delete() on the message that crossed threshold, leaving earlier duplicates in the window undeleted. Now deletes every message_id in the similar-message window via context.bot.delete_message. --- src/bot/handlers/duplicate_spam.py | 31 +++++++++++++++++++----------- tests/test_duplicate_spam.py | 20 ++++++++++++------- 2 files changed, 33 insertions(+), 18 deletions(-) diff --git a/src/bot/handlers/duplicate_spam.py b/src/bot/handlers/duplicate_spam.py index e6844eb..a8fb293 100644 --- a/src/bot/handlers/duplicate_spam.py +++ b/src/bot/handlers/duplicate_spam.py @@ -132,7 +132,10 @@ async def handle_duplicate_spam( dq = _get_recent_messages(context, group_config.group_id, user.id) _prune_old_messages(dq, group_config.duplicate_spam_window_seconds, now) - similar_count = count_similar_in_window(dq, normalized, group_config.duplicate_spam_similarity) + similar_messages = [ + m for m in dq + if is_similar(normalized, m.normalized_text, group_config.duplicate_spam_similarity) + ] dq.append( RecentMessage( @@ -142,10 +145,10 @@ async def handle_duplicate_spam( ) ) - if similar_count < group_config.duplicate_spam_threshold - 1: + if len(similar_messages) < group_config.duplicate_spam_threshold - 1: return - total_count = similar_count + 1 + total_count = len(similar_messages) + 1 user_mention = get_user_mention(user) logger.info( @@ -153,14 +156,20 @@ async def handle_duplicate_spam( f"group_id={group_config.group_id}, count={total_count}" ) - try: - await update.message.delete() - logger.info(f"Deleted duplicate spam from user_id={user.id}") - except Exception: - logger.error( - f"Failed to delete duplicate spam: user_id={user.id}", - exc_info=True, - ) + message_ids = [m.message_id for m in similar_messages] + [update.message.message_id] + for message_id in message_ids: + try: + await context.bot.delete_message( + chat_id=group_config.group_id, message_id=message_id + ) + logger.info( + f"Deleted duplicate spam message_id={message_id} from user_id={user.id}" + ) + except Exception: + logger.error( + f"Failed to delete duplicate spam message_id={message_id}: user_id={user.id}", + exc_info=True, + ) await _enforce_restriction(context, group_config, user, user_mention, total_count) diff --git a/tests/test_duplicate_spam.py b/tests/test_duplicate_spam.py index 404fd69..39115a8 100644 --- a/tests/test_duplicate_spam.py +++ b/tests/test_duplicate_spam.py @@ -169,7 +169,6 @@ def mock_update(self): update.message.text = "Barangkali di sini ada yang sedang mencari kerja bisa menghubungi saya" update.message.caption = None update.message.message_id = 100 - update.message.delete = AsyncMock() update.effective_chat = MagicMock(spec=Chat) update.effective_chat.id = -100 return update @@ -181,6 +180,7 @@ def mock_context(self): context.bot = MagicMock() context.bot.restrict_chat_member = AsyncMock() context.bot.send_message = AsyncMock() + context.bot.delete_message = AsyncMock() return context async def test_skips_no_message(self, mock_context, group_config): @@ -291,7 +291,9 @@ async def test_second_message_triggers_restriction(self, mock_update, mock_conte with pytest.raises(ApplicationHandlerStop): await handle_duplicate_spam(mock_update, mock_context) - mock_update.message.delete.assert_called_once() + assert mock_context.bot.delete_message.call_count == 2 + deleted_ids = {c.kwargs["message_id"] for c in mock_context.bot.delete_message.call_args_list} + assert deleted_ids == {99, 100} mock_context.bot.restrict_chat_member.assert_called_once() mock_context.bot.send_message.assert_called_once() @@ -309,7 +311,7 @@ async def test_uses_caption_when_no_text(self, mock_update, mock_context, group_ with pytest.raises(ApplicationHandlerStop): await handle_duplicate_spam(mock_update, mock_context) - mock_update.message.delete.assert_called_once() + assert mock_context.bot.delete_message.call_count == 2 async def test_expired_messages_not_counted(self, mock_update, mock_context, group_config): old = datetime.now(UTC) - timedelta(seconds=200) @@ -322,7 +324,7 @@ async def test_expired_messages_not_counted(self, mock_update, mock_context, gro with patch("bot.handlers.duplicate_spam.get_group_config_for_update", return_value=group_config): await handle_duplicate_spam(mock_update, mock_context) - mock_update.message.delete.assert_not_called() + mock_context.bot.delete_message.assert_not_called() async def test_different_messages_not_counted(self, mock_update, mock_context, group_config): now = datetime.now(UTC) @@ -335,10 +337,10 @@ async def test_different_messages_not_counted(self, mock_update, mock_context, g with patch("bot.handlers.duplicate_spam.get_group_config_for_update", return_value=group_config): await handle_duplicate_spam(mock_update, mock_context) - mock_update.message.delete.assert_not_called() + mock_context.bot.delete_message.assert_not_called() async def test_delete_failure_continues(self, mock_update, mock_context, group_config): - mock_update.message.delete = AsyncMock(side_effect=Exception("Delete failed")) + mock_context.bot.delete_message = AsyncMock(side_effect=Exception("Delete failed")) now = datetime.now(UTC) norm = normalize_text(mock_update.message.text) existing_dq = deque([ @@ -395,5 +397,9 @@ async def test_third_message_also_triggers(self, mock_update, mock_context, grou with pytest.raises(ApplicationHandlerStop): await handle_duplicate_spam(mock_update, mock_context) - mock_update.message.delete.assert_called_once() + # Regression check: all prior duplicate messages must be deleted, + # not just the one that tripped the threshold. + assert mock_context.bot.delete_message.call_count == 3 + deleted_ids = {c.kwargs["message_id"] for c in mock_context.bot.delete_message.call_args_list} + assert deleted_ids == {98, 99, 100} mock_context.bot.restrict_chat_member.assert_called_once() From 3e43198aa82a22a42ffafd6fc7b1dc2b4977e26b Mon Sep 17 00:00:00 2001 From: Rezha Julio Date: Fri, 3 Jul 2026 14:38:13 +0700 Subject: [PATCH 2/3] refactor(anti-spam): drop dead count_similar_in_window helper Superseded by the similar_messages list built inline in handle_duplicate_spam, which needs the message_ids for deletion, not just a count. --- src/bot/handlers/duplicate_spam.py | 7 ------- tests/test_duplicate_spam.py | 19 ------------------- 2 files changed, 26 deletions(-) diff --git a/src/bot/handlers/duplicate_spam.py b/src/bot/handlers/duplicate_spam.py index a8fb293..7341d24 100644 --- a/src/bot/handlers/duplicate_spam.py +++ b/src/bot/handlers/duplicate_spam.py @@ -86,13 +86,6 @@ def _prune_old_messages( dq.popleft() -def count_similar_in_window( - dq: deque[RecentMessage], normalized: str, threshold: float = 0.95 -) -> int: - """Count how many messages in the deque are similar to the given text.""" - return sum(1 for m in dq if is_similar(normalized, m.normalized_text, threshold)) - - async def handle_duplicate_spam( update: Update, context: ContextTypes.DEFAULT_TYPE ) -> None: diff --git a/tests/test_duplicate_spam.py b/tests/test_duplicate_spam.py index 39115a8..d2cb6fc 100644 --- a/tests/test_duplicate_spam.py +++ b/tests/test_duplicate_spam.py @@ -13,7 +13,6 @@ RecentMessage, _get_recent_messages, _prune_old_messages, - count_similar_in_window, handle_duplicate_spam, is_similar, normalize_text, @@ -104,24 +103,6 @@ def test_empty_deque(self): assert len(dq) == 0 -class TestCountSimilarInWindow: - """Tests for the count_similar_in_window function.""" - - def test_counts_similar(self): - dq = deque([ - RecentMessage(timestamp=datetime.now(UTC), normalized_text="spam message here", message_id=1), - RecentMessage(timestamp=datetime.now(UTC), normalized_text="spam message here", message_id=2), - RecentMessage(timestamp=datetime.now(UTC), normalized_text="different message", message_id=3), - ]) - assert count_similar_in_window(dq, "spam message here") == 2 - - def test_no_similar(self): - dq = deque([ - RecentMessage(timestamp=datetime.now(UTC), normalized_text="hello world foo bar", message_id=1), - ]) - assert count_similar_in_window(dq, "completely different text here") == 0 - - class TestGetRecentMessages: """Tests for the _get_recent_messages function.""" From 1a6ef5e4cb0dee7b9018df8e0e93bb5853f3b37f Mon Sep 17 00:00:00 2001 From: Rezha Julio Date: Fri, 3 Jul 2026 14:46:48 +0700 Subject: [PATCH 3/3] fix(anti-spam): don't retry delete on already-deleted duplicate messages Deque entries stayed forever, so every subsequent duplicate message in the same window re-attempted delete_message on already-deleted message_ids, spamming error logs until the window expired. Track delete_attempted per entry and skip it. --- src/bot/handlers/duplicate_spam.py | 26 +++++++++++++++----------- tests/test_duplicate_spam.py | 24 ++++++++++++++++++++++++ 2 files changed, 39 insertions(+), 11 deletions(-) diff --git a/src/bot/handlers/duplicate_spam.py b/src/bot/handlers/duplicate_spam.py index 7341d24..0199765 100644 --- a/src/bot/handlers/duplicate_spam.py +++ b/src/bot/handlers/duplicate_spam.py @@ -42,6 +42,7 @@ class RecentMessage: timestamp: datetime normalized_text: str message_id: int + delete_attempted: bool = False def normalize_text(text: str) -> str: @@ -130,13 +131,12 @@ async def handle_duplicate_spam( if is_similar(normalized, m.normalized_text, group_config.duplicate_spam_similarity) ] - dq.append( - RecentMessage( - timestamp=now, - normalized_text=normalized, - message_id=update.message.message_id, - ) + current_message = RecentMessage( + timestamp=now, + normalized_text=normalized, + message_id=update.message.message_id, ) + dq.append(current_message) if len(similar_messages) < group_config.duplicate_spam_threshold - 1: return @@ -149,20 +149,24 @@ async def handle_duplicate_spam( f"group_id={group_config.group_id}, count={total_count}" ) - message_ids = [m.message_id for m in similar_messages] + [update.message.message_id] - for message_id in message_ids: + pending_deletes = [m for m in similar_messages if not m.delete_attempted] + pending_deletes.append(current_message) + + for m in pending_deletes: try: await context.bot.delete_message( - chat_id=group_config.group_id, message_id=message_id + chat_id=group_config.group_id, message_id=m.message_id ) logger.info( - f"Deleted duplicate spam message_id={message_id} from user_id={user.id}" + f"Deleted duplicate spam message_id={m.message_id} from user_id={user.id}" ) except Exception: logger.error( - f"Failed to delete duplicate spam message_id={message_id}: user_id={user.id}", + f"Failed to delete duplicate spam message_id={m.message_id}: user_id={user.id}", exc_info=True, ) + finally: + m.delete_attempted = True await _enforce_restriction(context, group_config, user, user_mention, total_count) diff --git a/tests/test_duplicate_spam.py b/tests/test_duplicate_spam.py index d2cb6fc..9686e3e 100644 --- a/tests/test_duplicate_spam.py +++ b/tests/test_duplicate_spam.py @@ -384,3 +384,27 @@ async def test_third_message_also_triggers(self, mock_update, mock_context, grou deleted_ids = {c.kwargs["message_id"] for c in mock_context.bot.delete_message.call_args_list} assert deleted_ids == {98, 99, 100} mock_context.bot.restrict_chat_member.assert_called_once() + + async def test_does_not_retry_already_attempted_deletes( + self, mock_update, mock_context, group_config + ): + """Messages already deleted in a prior trigger must not be re-attempted.""" + now = datetime.now(UTC) + norm = normalize_text(mock_update.message.text) + existing_dq = deque([ + RecentMessage( + timestamp=now, normalized_text=norm, message_id=98, delete_attempted=True + ), + RecentMessage( + timestamp=now, normalized_text=norm, message_id=99, delete_attempted=True + ), + ]) + mock_context.bot_data[RECENT_MESSAGES_KEY] = {(-100, 42): existing_dq} + + with patch("bot.handlers.duplicate_spam.get_group_config_for_update", return_value=group_config): + with pytest.raises(ApplicationHandlerStop): + await handle_duplicate_spam(mock_update, mock_context) + + mock_context.bot.delete_message.assert_called_once_with( + chat_id=-100, message_id=100 + )