From 755a3851f78feec8f521cd2e7508b6462702261d Mon Sep 17 00:00:00 2001 From: Ali Khokhar Date: Sun, 5 Jul 2026 22:07:09 -0700 Subject: [PATCH] Use batch delete boundary for messaging (#996) ## Problem Messaging cleanup had two public queued delete paths. Command code could loop single-message deletes and bypass Telegram batch deletion. ## Changes | Before | After | | --- | --- | | Workflow code could call `queue_delete_message` or `queue_delete_messages`. | Workflow code calls only `queue_delete_messages`. | | Telegram `/clear` cleanup used one API request per message. | Telegram `/clear` cleanup uses `deleteMessages` in chunks of 100. | | The outbox dedupe key used Python process hashing. | The outbox dedupe key uses a stable SHA-based digest. | | Voice and smoke cleanup depended on the single-delete queue API. | Voice and smoke cleanup pass one-item delete lists. |

Greptile Summary

 This PR moves messaging cleanup to a list-based delete boundary. The main changes are: - `/clear` now sends collected message IDs through `queue_delete_messages`. - Telegram deletion uses `deleteMessages` in 100-message chunks with per-message fallback. - Discord keeps per-message deletion behind the list-based outbound API. - Delete-batch dedupe keys now use a stable SHA digest instead of Python process hashing. - Voice cleanup, smoke fakes, protocol tests, and messaging tests were updated for the new delete boundary.

Confidence Score: 5/5

Safe to merge with minimal risk. The change is well-scoped to the messaging delete boundary, keeps platform-specific best-effort behavior, addresses the batch-fallback concern, updates protocol consumers and tests, and includes the required version and lockfile bump. No files require special attention.

T-Rex T-Rex Logs

 **What T-Rex did** - No execution evidence is available for this session; no harness was created, no tests were run, and no artifacts were produced. T-Rex Ran code and verified through T-Rex

Important Files Changed

 | Filename | Overview | |----------|----------| | messaging/commands.py | Routes `/clear` cleanup through `queue_delete_messages` once per collected message set while preserving best-effort state cleanup. | | messaging/platforms/outbox.py | Removes single-delete queueing, snapshots delete batches, and uses a stable SHA digest for delete dedupe keys. | | messaging/platforms/telegram_io.py | Adds Telegram `deleteMessages` batching with 100-message chunks and per-message fallback when batch deletion fails. | | messaging/platforms/discord_io.py | Removes the public single queued-delete wrapper and backs queued deletion with the list-based outbox API. | | messaging/platforms/voice_flow.py | Changes shared voice cleanup call sites to submit one-item lists to the delete queue. | | messaging/platforms/ports.py | Narrows the outbound protocol to the list-based delete queue method. | | tests/messaging/test_telegram.py | Adds Telegram batch delete, chunking, and fallback coverage. | | tests/messaging/test_platform_outbox.py | Covers stable delete-batch dedupe keys and snapshotting mutable message ID lists before queueing. | | pyproject.toml | Bumps the package patch version for the production messaging changes. | | uv.lock | Updates the editable package version in the lockfile to match `pyproject.toml`. |

Sequence Diagram

 ```mermaid %%{init: {'theme': 'neutral'}}%% sequenceDiagram participant Command as /clear or voice cleanup participant Outbound as OutboundMessenger.queue_delete_messages participant Outbox as PlatformOutbox participant Telegram as TelegramMessenger participant Discord as DiscordMessenger participant API as Platform API Command->>Outbound: queue_delete_messages(chat_id, message_ids) Outbound->>Outbox: snapshot IDs and dedupe batch alt Telegram Outbox->>Telegram: delete_messages(chat_id, ids) loop chunks of 100 Telegram->>API: deleteMessages(chat_id, chunk) alt batch fails Telegram->>API: deleteMessage(chat_id, each id) end end else Discord Outbox->>Discord: delete_messages(chat_id, ids) loop each id Discord->>API: fetch_message + delete end end ``` ```mermaid %%{init: {'theme': 'base', 'themeVariables': {"darkMode": true, "background": "#0d1117", "primaryColor": "#21262d", "primaryTextColor": "#e6edf3", "primaryBorderColor": "#8b949e", "lineColor": "#8b949e", "textColor": "#e6edf3", "edgeLabelBackground": "#161b22", "actorBkg": "#21262d", "actorBorder": "#8b949e", "actorTextColor": "#e6edf3", "actorLineColor": "#8b949e", "signalColor": "#8b949e", "signalTextColor": "#e6edf3", "noteBkgColor": "#373320", "noteBorderColor": "#d4a72c", "noteTextColor": "#f0e6c0", "labelBoxBkgColor": "#21262d", "labelBoxBorderColor": "#8b949e", "labelTextColor": "#e6edf3", "loopTextColor": "#e6edf3", "activationBkgColor": "#30363d", "activationBorderColor": "#8b949e"}}}%% sequenceDiagram participant Command as /clear or voice cleanup participant Outbound as OutboundMessenger.queue_delete_messages participant Outbox as PlatformOutbox participant Telegram as TelegramMessenger participant Discord as DiscordMessenger participant API as Platform API Command->>Outbound: queue_delete_messages(chat_id, message_ids) Outbound->>Outbox: snapshot IDs and dedupe batch alt Telegram Outbox->>Telegram: delete_messages(chat_id, ids) loop chunks of 100 Telegram->>API: deleteMessages(chat_id, chunk) alt batch fails Telegram->>API: deleteMessage(chat_id, each id) end end else Discord Outbox->>Discord: delete_messages(chat_id, ids) loop each id Discord->>API: fetch_message + delete end end ```
Reviews (2): Last reviewed commit: ["Preserve Telegram batch delete fallback"](https://github.com/alishahryar1/free-claude-code/commit/87090edcd95c4014cae37c2f540d9bfa4eba8962) | [Re-trigger Greptile](https://app.greptile.com/api/retrigger?id=41993526) --- ARCHITECTURE.md | 13 +- messaging/commands.py | 38 ++---- messaging/platforms/discord.py | 2 +- messaging/platforms/discord_io.py | 10 -- messaging/platforms/outbox.py | 35 ++---- messaging/platforms/ports.py | 7 -- messaging/platforms/telegram.py | 2 +- messaging/platforms/telegram_io.py | 77 +++++++----- messaging/platforms/voice_flow.py | 16 +-- pyproject.toml | 2 +- smoke/lib/e2e.py | 10 +- tests/api/test_app_lifespan_and_errors.py | 1 - tests/conftest.py | 11 +- tests/contracts/test_import_boundaries.py | 7 ++ tests/messaging/test_handler.py | 54 +++++---- tests/messaging/test_messaging.py | 1 - tests/messaging/test_platform_outbox.py | 45 ++++--- tests/messaging/test_platform_voice_flow.py | 18 +-- tests/messaging/test_telegram.py | 127 ++++++++++++++++++++ tests/messaging/test_telegram_edge_cases.py | 17 +++ uv.lock | 2 +- 21 files changed, 303 insertions(+), 192 deletions(-) diff --git a/ARCHITECTURE.md b/ARCHITECTURE.md index 32af75d9..a64b7799 100644 --- a/ARCHITECTURE.md +++ b/ARCHITECTURE.md @@ -571,8 +571,10 @@ Outbound SDK calls live in [messaging/platforms/telegram_io.py](messaging/platforms/telegram_io.py) and [messaging/platforms/discord_io.py](messaging/platforms/discord_io.py). Shared delivery policy lives in [messaging/platforms/outbox.py](messaging/platforms/outbox.py), -which owns queued send/edit/delete, dedup keys, limiter delegation, and -fire-and-forget behavior. +which owns queued send/edit/list-based delete, dedup keys, limiter delegation, +and fire-and-forget behavior. Workflow and command code request deletion of +message ID lists; platform IO decides whether to use native batch deletion +(Telegram) or internal per-message deletion (Discord). Shared voice-note orchestration lives in [messaging/platforms/voice_flow.py](messaging/platforms/voice_flow.py), which owns pending voice registration, temp-file cleanup, transcription, cancellation, error @@ -628,9 +630,10 @@ APIs to runtime code. Debounced atomic writes live in [messaging/session/persistence.py](messaging/session/persistence.py), and per-chat message ID tracking for `/clear` lives in [messaging/session/message_log.py](messaging/session/message_log.py). `/clear` -guarantees FCC state cleanup and tries each tracked platform delete, but -Discord/Telegram can still reject individual message deletions for platform -reasons such as permissions, age, or missing messages. +guarantees FCC state cleanup and tries tracked platform deletes through the +list-based outbound delete port, but Discord/Telegram can still reject +individual message deletions for platform reasons such as permissions, age, or +missing messages. ```mermaid sequenceDiagram diff --git a/messaging/commands.py b/messaging/commands.py index 5965296e..b4887ec5 100644 --- a/messaging/commands.py +++ b/messaging/commands.py @@ -3,21 +3,12 @@ Commands depend on MessagingCommandContext instead of the concrete workflow. """ -from dataclasses import dataclass - from loguru import logger from .command_context import MessagingCommandContext from .models import IncomingMessage -@dataclass(frozen=True, slots=True) -class MessageDeleteReport: - attempted: int - deleted: int - failed: int - - async def handle_stop_command( handler: MessagingCommandContext, incoming: IncomingMessage ) -> None: @@ -95,10 +86,10 @@ async def handle_stats_command( async def _delete_message_ids( handler: MessagingCommandContext, chat_id: str, msg_ids: set[str] -) -> MessageDeleteReport: +) -> None: """Best-effort delete messages by ID. Sorts numeric IDs descending.""" if not msg_ids: - return MessageDeleteReport(attempted=0, deleted=0, failed=0) + return def _as_int(s: str) -> int | None: try: @@ -117,28 +108,23 @@ async def _delete_message_ids( numeric.sort(reverse=True) ordered = [mid for _, mid in numeric] + non_numeric - deleted = 0 failed = 0 - for mid in ordered: - try: - await handler.outbound.queue_delete_message( - chat_id, mid, fire_and_forget=False - ) - deleted += 1 - except Exception as e: - failed += 1 - logger.debug( - "Message delete failed for chat {}: {}", chat_id, type(e).__name__ - ) + try: + await handler.outbound.queue_delete_messages( + chat_id, + ordered, + fire_and_forget=False, + ) + except Exception as e: + failed = len(ordered) + logger.debug("Message delete failed for chat {}: {}", chat_id, type(e).__name__) if ordered: logger.info( - "Clear delete attempted={} deleted={} failed={}", + "Clear delete attempted={} failed={}", len(ordered), - deleted, failed, ) - return MessageDeleteReport(attempted=len(ordered), deleted=deleted, failed=failed) async def _handle_clear_branch( diff --git a/messaging/platforms/discord.py b/messaging/platforms/discord.py index c7aab7d8..d384b024 100644 --- a/messaging/platforms/discord.py +++ b/messaging/platforms/discord.py @@ -141,7 +141,7 @@ class DiscordRuntime: discord_voice_request_from_event(message, attachment, channel_id), message_handler=self._message_handler, queue_send_message=self.outbound.queue_send_message, - queue_delete_message=self.outbound.queue_delete_message, + queue_delete_messages=self.outbound.queue_delete_messages, ) async def _on_discord_message(self, message: Any) -> None: diff --git a/messaging/platforms/discord_io.py b/messaging/platforms/discord_io.py index 4aeddb08..85498639 100644 --- a/messaging/platforms/discord_io.py +++ b/messaging/platforms/discord_io.py @@ -35,7 +35,6 @@ class DiscordMessenger: get_limiter=get_limiter, send=self.send_message, edit=self.edit_message, - delete=self.delete_message, delete_many=self.delete_messages, ) @@ -146,15 +145,6 @@ class DiscordMessenger: fire_and_forget, ) - async def queue_delete_message( - self, - chat_id: str, - message_id: str, - fire_and_forget: bool = True, - ) -> None: - """Queue a Discord delete.""" - await self._outbox.queue_delete_message(chat_id, message_id, fire_and_forget) - async def queue_delete_messages( self, chat_id: str, diff --git a/messaging/platforms/outbox.py b/messaging/platforms/outbox.py index 6ed85f01..6afeef58 100644 --- a/messaging/platforms/outbox.py +++ b/messaging/platforms/outbox.py @@ -1,6 +1,7 @@ """Shared queued delivery helper for messaging platforms.""" import asyncio +import hashlib from collections.abc import Awaitable, Callable from typing import Any, cast @@ -9,7 +10,6 @@ SendOperation = Callable[ Awaitable[str], ] EditOperation = Callable[[str, str, str, str | None], Awaitable[None]] -DeleteOperation = Callable[[str, str], Awaitable[None]] DeleteManyOperation = Callable[[str, list[str]], Awaitable[None]] LimiterGetter = Callable[[], Any | None] @@ -23,13 +23,11 @@ class PlatformOutbox: get_limiter: LimiterGetter, send: SendOperation, edit: EditOperation, - delete: DeleteOperation, delete_many: DeleteManyOperation, ) -> None: self._get_limiter = get_limiter self._send = send self._edit = edit - self._delete = delete self._delete_many = delete_many async def queue_send_message( @@ -89,27 +87,6 @@ class PlatformOutbox: else: await limiter.enqueue(_edit, dedup_key=dedup_key) - async def queue_delete_message( - self, - chat_id: str, - message_id: str, - fire_and_forget: bool = True, - ) -> None: - """Queue or immediately delete a platform message.""" - limiter = self._get_limiter() - if limiter is None: - await self._delete(chat_id, message_id) - return - - async def _delete() -> None: - await self._delete(chat_id, message_id) - - dedup_key = f"del:{chat_id}:{message_id}" - if fire_and_forget: - limiter.fire_and_forget(_delete, dedup_key=dedup_key) - else: - await limiter.enqueue(_delete, dedup_key=dedup_key) - async def queue_delete_messages( self, chat_id: str, @@ -117,18 +94,20 @@ class PlatformOutbox: fire_and_forget: bool = True, ) -> None: """Queue or immediately bulk-delete platform messages.""" - if not message_ids: + ids_snapshot = tuple(str(message_id) for message_id in message_ids) + if not ids_snapshot: return limiter = self._get_limiter() if limiter is None: - await self._delete_many(chat_id, message_ids) + await self._delete_many(chat_id, list(ids_snapshot)) return async def _delete_many() -> None: - await self._delete_many(chat_id, message_ids) + await self._delete_many(chat_id, list(ids_snapshot)) - dedup_key = f"del_bulk:{chat_id}:{hash(tuple(message_ids))}" + digest = hashlib.sha256("\x1f".join(ids_snapshot).encode()).hexdigest()[:16] + dedup_key = f"del_bulk:{chat_id}:{digest}" if fire_and_forget: limiter.fire_and_forget(_delete_many, dedup_key=dedup_key) else: diff --git a/messaging/platforms/ports.py b/messaging/platforms/ports.py index 35dcfa50..0a100758 100644 --- a/messaging/platforms/ports.py +++ b/messaging/platforms/ports.py @@ -49,13 +49,6 @@ class OutboundMessenger(Protocol): fire_and_forget: bool = True, ) -> None: ... - async def queue_delete_message( - self, - chat_id: str, - message_id: str, - fire_and_forget: bool = True, - ) -> None: ... - async def queue_delete_messages( self, chat_id: str, diff --git a/messaging/platforms/telegram.py b/messaging/platforms/telegram.py index b12201b2..246218c3 100644 --- a/messaging/platforms/telegram.py +++ b/messaging/platforms/telegram.py @@ -270,5 +270,5 @@ class TelegramRuntime: request, message_handler=self._message_handler, queue_send_message=self.outbound.queue_send_message, - queue_delete_message=self.outbound.queue_delete_message, + queue_delete_messages=self.outbound.queue_delete_messages, ) diff --git a/messaging/platforms/telegram_io.py b/messaging/platforms/telegram_io.py index ff71937f..8bccef70 100644 --- a/messaging/platforms/telegram_io.py +++ b/messaging/platforms/telegram_io.py @@ -9,6 +9,8 @@ from loguru import logger from .outbox import PlatformOutbox +TELEGRAM_DELETE_MESSAGES_BATCH_SIZE = 100 + TelegramNetworkError: type[BaseException] TelegramRetryAfter: type[BaseException] TelegramBaseError: type[BaseException] @@ -49,12 +51,15 @@ class TelegramMessenger: get_limiter=get_limiter, send=self.send_message, edit=self.edit_message, - delete=self.delete_message, delete_many=self.delete_messages, ) async def _with_retry( - self, func: Callable[..., Awaitable[Any]], *args: Any, **kwargs: Any + self, + func: Callable[..., Awaitable[Any]], + *args: Any, + suppress_known_message_errors: bool = True, + **kwargs: Any, ) -> Any: """Execute a Telegram API call with the platform retry policy.""" max_retries = 3 @@ -63,7 +68,9 @@ class TelegramMessenger: return await func(*args, **kwargs) except (TimeoutError, TelegramNetworkError) as e: if "Message is not modified" in str(e): - return None + if suppress_known_message_errors: + return None + raise if attempt < max_retries - 1: wait_time = 2**attempt logger.warning( @@ -95,7 +102,9 @@ class TelegramMessenger: except TelegramBaseError as e: err_lower = str(e).lower() if "message is not modified" in err_lower: - return None + if suppress_known_message_errors: + return None + raise if any( x in err_lower for x in [ @@ -106,7 +115,9 @@ class TelegramMessenger: "not enough rights to delete", ] ): - return None + if suppress_known_message_errors: + return None + raise if "Can't parse entities" in str(e) and kwargs.get("parse_mode"): logger.warning("Markdown failed, retrying without parse_mode") kwargs["parse_mode"] = None @@ -183,23 +194,40 @@ class TelegramMessenger: raise RuntimeError("Telegram application or bot not initialized") bot = app.bot - if hasattr(bot, "delete_messages"): - - async def _do_bulk() -> None: - mids: list[int] = [] - for mid in message_ids: - try: - mids.append(int(mid)) - except Exception: - continue - if mids: - await bot.delete_messages(chat_id=chat_id, message_ids=mids) - - await self._with_retry(_do_bulk) + mids: list[int] = [] + for mid in message_ids: + try: + mids.append(int(mid)) + except Exception: + continue + if not mids: return - for mid in message_ids: - await self.delete_message(chat_id, mid) + if hasattr(bot, "delete_messages"): + for start in range(0, len(mids), TELEGRAM_DELETE_MESSAGES_BATCH_SIZE): + chunk = mids[start : start + TELEGRAM_DELETE_MESSAGES_BATCH_SIZE] + chunk_snapshot = tuple(chunk) + + async def _do_bulk(ids: tuple[int, ...] = chunk_snapshot) -> None: + await bot.delete_messages(chat_id=chat_id, message_ids=list(ids)) + + try: + await self._with_retry( + _do_bulk, + suppress_known_message_errors=False, + ) + except Exception as e: + logger.debug( + "Telegram bulk delete failed for chat {}: {}; falling back", + chat_id, + type(e).__name__, + ) + for mid in chunk: + await self.delete_message(chat_id, str(mid)) + return + + for mid in mids: + await self.delete_message(chat_id, str(mid)) async def queue_send_message( self, @@ -237,15 +265,6 @@ class TelegramMessenger: fire_and_forget, ) - async def queue_delete_message( - self, - chat_id: str, - message_id: str, - fire_and_forget: bool = True, - ) -> None: - """Queue a Telegram delete.""" - await self._outbox.queue_delete_message(chat_id, message_id, fire_and_forget) - async def queue_delete_messages( self, chat_id: str, diff --git a/messaging/platforms/voice_flow.py b/messaging/platforms/voice_flow.py index fa7ade3c..a06ed5d5 100644 --- a/messaging/platforms/voice_flow.py +++ b/messaging/platforms/voice_flow.py @@ -22,7 +22,7 @@ VOICE_TRANSCRIPTION_ERROR_MESSAGE = ( MessageHandler = Callable[[IncomingMessage], Awaitable[None]] QueueSend = Callable[..., Awaitable[str | None]] -QueueDelete = Callable[..., Awaitable[None]] +QueueDeleteMany = Callable[..., Awaitable[None]] @dataclass(frozen=True) @@ -148,7 +148,7 @@ class VoiceNoteFlow: *, message_handler: MessageHandler | None, queue_send_message: QueueSend, - queue_delete_message: QueueDelete, + queue_delete_messages: QueueDeleteMany, ) -> bool: """Transcribe a voice note and hand the resulting turn to messaging.""" if await self.reply_if_disabled(request.reply_text): @@ -192,7 +192,7 @@ class VoiceNoteFlow: request.chat_id, request.message_id, ): - await queue_delete_message(request.chat_id, status_msg_id_text) + await queue_delete_messages(request.chat_id, [status_msg_id_text]) return True await self.complete_pending_voice( @@ -222,7 +222,7 @@ class VoiceNoteFlow: await self._clear_failed_pending_voice( request, status_msg_id_text, - queue_delete_message, + queue_delete_messages, handed_off=handed_off, ) await request.reply_text(format_user_error_preview(e)) @@ -231,7 +231,7 @@ class VoiceNoteFlow: await self._clear_failed_pending_voice( request, status_msg_id_text, - queue_delete_message, + queue_delete_messages, handed_off=handed_off, ) await request.reply_text(format_user_error_preview(e)) @@ -240,7 +240,7 @@ class VoiceNoteFlow: await self._clear_failed_pending_voice( request, status_msg_id_text, - queue_delete_message, + queue_delete_messages, handed_off=handed_off, ) if self._log_api_error_tracebacks: @@ -260,7 +260,7 @@ class VoiceNoteFlow: self, request: VoiceNoteRequest, status_msg_id: str, - queue_delete_message: QueueDelete, + queue_delete_messages: QueueDeleteMany, *, handed_off: bool, ) -> None: @@ -271,7 +271,7 @@ class VoiceNoteFlow: ) if not handed_off: with contextlib.suppress(Exception): - await queue_delete_message(request.chat_id, status_msg_id) + await queue_delete_messages(request.chat_id, [status_msg_id]) def _log_transcription(self, request: VoiceNoteRequest, transcribed: str) -> None: label = request.platform.upper() diff --git a/pyproject.toml b/pyproject.toml index 1f75a1e6..f294f105 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -4,7 +4,7 @@ build-backend = "hatchling.build" [project] name = "free-claude-code" -version = "3.4.1" +version = "3.4.2" description = "Middleware between Claude Code CLI (Anthropic API) and NVIDIA NIM" readme = "README.md" requires-python = ">=3.14.0" diff --git a/smoke/lib/e2e.py b/smoke/lib/e2e.py index 7fe6de26..be6f17dc 100644 --- a/smoke/lib/e2e.py +++ b/smoke/lib/e2e.py @@ -408,14 +408,6 @@ class FakePlatform: ) -> None: await self.edit_message(chat_id, message_id, text, parse_mode=parse_mode) - async def queue_delete_message( - self, - chat_id: str, - message_id: str, - fire_and_forget: bool = True, - ) -> None: - await self.delete_message(chat_id, message_id) - async def queue_delete_messages( self, chat_id: str, @@ -423,7 +415,7 @@ class FakePlatform: fire_and_forget: bool = True, ) -> None: for message_id in message_ids: - await self.queue_delete_message(chat_id, message_id, fire_and_forget=False) + await self.delete_message(chat_id, message_id) def register_pending_voice( self, chat_id: str, voice_message_id: str, status_message_id: str diff --git a/tests/api/test_app_lifespan_and_errors.py b/tests/api/test_app_lifespan_and_errors.py index e4c5ab59..65829068 100644 --- a/tests/api/test_app_lifespan_and_errors.py +++ b/tests/api/test_app_lifespan_and_errors.py @@ -51,7 +51,6 @@ def _fake_messaging_components(runtime: MagicMock | None = None) -> SimpleNamesp outbound = MagicMock() outbound.queue_send_message = AsyncMock(return_value="msg") outbound.queue_edit_message = AsyncMock() - outbound.queue_delete_message = AsyncMock() outbound.queue_delete_messages = AsyncMock() outbound.fire_and_forget = MagicMock() return SimpleNamespace( diff --git a/tests/conftest.py b/tests/conftest.py index 848b3904..81c42b67 100644 --- a/tests/conftest.py +++ b/tests/conftest.py @@ -116,16 +116,7 @@ def mock_platform(): platform.delete_message = AsyncMock() platform.queue_send_message = AsyncMock(return_value="msg_123") platform.queue_edit_message = AsyncMock() - platform.queue_delete_message = AsyncMock() - - async def _queue_delete_messages( - chat_id: str, message_ids: list[str], *, fire_and_forget: bool = True - ) -> None: - qdm = platform.queue_delete_message - for mid in message_ids: - await qdm(chat_id, mid, fire_and_forget=fire_and_forget) - - platform.queue_delete_messages = AsyncMock(side_effect=_queue_delete_messages) + platform.queue_delete_messages = AsyncMock() platform.cancel_pending_voice = AsyncMock(return_value=None) def _fire_and_forget(task): diff --git a/tests/contracts/test_import_boundaries.py b/tests/contracts/test_import_boundaries.py index b33b2ecb..4f28c3f5 100644 --- a/tests/contracts/test_import_boundaries.py +++ b/tests/contracts/test_import_boundaries.py @@ -551,6 +551,12 @@ def test_messaging_platforms_use_shared_outbox_and_voice_flow() -> None: assert (platforms_root / "ports.py").exists() assert (platforms_root / "outbox.py").exists() assert (platforms_root / "voice_flow.py").exists() + assert "def queue_delete_message(" not in (platforms_root / "ports.py").read_text( + encoding="utf-8" + ) + assert "def queue_delete_message(" not in (platforms_root / "outbox.py").read_text( + encoding="utf-8" + ) for runtime in { platforms_root / "telegram.py", @@ -568,6 +574,7 @@ def test_messaging_platforms_use_shared_outbox_and_voice_flow() -> None: }: text = messenger.read_text(encoding="utf-8") assert "PlatformOutbox" in text + assert "def queue_delete_message(" not in text def test_cli_surfaces_are_explicit_launchers_and_managed_claude() -> None: diff --git a/tests/messaging/test_handler.py b/tests/messaging/test_handler.py index 708b8b92..74faa076 100644 --- a/tests/messaging/test_handler.py +++ b/tests/messaging/test_handler.py @@ -558,11 +558,11 @@ async def test_handle_message_clear_command_stops_deletes_and_wipes_state( events.append("stop") return 0 - async def _del(chat_id, message_id, fire_and_forget=True): - events.append(f"del:{chat_id}:{message_id}:{fire_and_forget}") + async def _del_many(chat_id, message_ids, fire_and_forget=True): + events.append(("del", chat_id, tuple(message_ids), fire_and_forget)) handler.stop_all_tasks = AsyncMock(side_effect=_stop) - mock_platform.queue_delete_message = AsyncMock(side_effect=_del) + mock_platform.queue_delete_messages = AsyncMock(side_effect=_del_many) incoming = incoming_message_factory( text="/clear", chat_id="chat_1", message_id="150" @@ -570,9 +570,9 @@ async def test_handle_message_clear_command_stops_deletes_and_wipes_state( await handler.handle_message(incoming) assert events and events[0] == "stop" - deleted_ids = {e.split(":")[2] for e in events[1:]} + deleted_ids = set(events[1][2]) assert deleted_ids == {"100", "101", "150"} - assert all(e.endswith(":False") for e in events[1:]) + assert events[1][3] is False mock_session_store.clear_all.assert_called_once() assert handler.tree_queue.get_tree_count() == 0 @@ -591,9 +591,9 @@ async def test_handle_message_clear_command_with_mention( await handler.handle_message(incoming) handler.stop_all_tasks.assert_called_once() - mock_platform.queue_delete_message.assert_called_once_with( + mock_platform.queue_delete_messages.assert_called_once_with( "chat_1", - "10", + ["10"], fire_and_forget=False, ) mock_session_store.clear_all.assert_called_once() @@ -611,8 +611,11 @@ async def test_handle_message_clear_command_deletes_message_log_ids( ) await handler.handle_message(incoming) - deleted = {c.args[1] for c in mock_platform.queue_delete_message.call_args_list} - assert deleted == {"42", "43", "150"} + mock_platform.queue_delete_messages.assert_called_once_with( + "chat_1", + ["150", "43", "42"], + fire_and_forget=False, + ) @pytest.mark.asyncio @@ -622,19 +625,21 @@ async def test_handle_message_clear_command_continues_after_delete_failure( handler.stop_all_tasks = AsyncMock(return_value=0) mock_session_store.get_message_ids_for_chat.return_value = ["41", "42", "43"] - async def delete_message(chat_id, message_id, fire_and_forget=True): - if message_id == "42": - raise RuntimeError("platform rejected delete") + async def delete_messages(chat_id, message_ids, fire_and_forget=True): + raise RuntimeError("platform rejected delete") - mock_platform.queue_delete_message = AsyncMock(side_effect=delete_message) + mock_platform.queue_delete_messages = AsyncMock(side_effect=delete_messages) incoming = incoming_message_factory( text="/clear", chat_id="chat_1", message_id="150" ) await handler.handle_message(incoming) - deleted = [c.args[1] for c in mock_platform.queue_delete_message.call_args_list] - assert deleted == ["150", "43", "42", "41"] + mock_platform.queue_delete_messages.assert_called_once_with( + "chat_1", + ["150", "43", "42", "41"], + fire_and_forget=False, + ) mock_session_store.clear_all.assert_called_once() @@ -666,10 +671,10 @@ async def test_handle_message_clear_command_reply_clears_branch( deleted_ids = [] - async def _capture_delete(chat_id, message_id, fire_and_forget=True): - deleted_ids.append(message_id) + async def _capture_delete(chat_id, message_ids, fire_and_forget=True): + deleted_ids.extend(message_ids) - mock_platform.queue_delete_message = AsyncMock(side_effect=_capture_delete) + mock_platform.queue_delete_messages = AsyncMock(side_effect=_capture_delete) incoming = incoming_message_factory( text="/clear", @@ -723,10 +728,10 @@ async def test_handle_message_clear_command_reply_to_root_clears_tree( deleted_ids = [] - async def _capture_delete(chat_id, message_id, fire_and_forget=True): - deleted_ids.append(message_id) + async def _capture_delete(chat_id, message_ids, fire_and_forget=True): + deleted_ids.extend(message_ids) - mock_platform.queue_delete_message = AsyncMock(side_effect=_capture_delete) + mock_platform.queue_delete_messages = AsyncMock(side_effect=_capture_delete) incoming = incoming_message_factory( text="/clear", @@ -800,13 +805,12 @@ async def test_handle_message_clear_command_reply_pending_voice_cancels( return None mock_platform.cancel_pending_voice = AsyncMock(side_effect=cancel_pending) - mock_platform.queue_delete_message = AsyncMock() deleted_ids = [] - async def _capture_delete(chat_id, message_id, fire_and_forget=True): - deleted_ids.append(message_id) + async def _capture_delete(chat_id, message_ids, fire_and_forget=True): + deleted_ids.extend(message_ids) - mock_platform.queue_delete_message = AsyncMock(side_effect=_capture_delete) + mock_platform.queue_delete_messages = AsyncMock(side_effect=_capture_delete) incoming = incoming_message_factory( text="/clear", diff --git a/tests/messaging/test_messaging.py b/tests/messaging/test_messaging.py index 03deab44..794b4471 100644 --- a/tests/messaging/test_messaging.py +++ b/tests/messaging/test_messaging.py @@ -58,7 +58,6 @@ class TestMessagingPorts: outbound = MagicMock() outbound.queue_send_message = AsyncMock() outbound.queue_edit_message = AsyncMock() - outbound.queue_delete_message = AsyncMock() outbound.queue_delete_messages = AsyncMock() outbound.fire_and_forget = MagicMock() components = MessagingPlatformComponents( diff --git a/tests/messaging/test_platform_outbox.py b/tests/messaging/test_platform_outbox.py index b0f26ce0..973c7bb6 100644 --- a/tests/messaging/test_platform_outbox.py +++ b/tests/messaging/test_platform_outbox.py @@ -5,7 +5,7 @@ import pytest from messaging.platforms.outbox import PlatformOutbox -def _noop_outbox(*, limiter=None) -> PlatformOutbox: +def _noop_outbox(*, limiter=None, delete_many=None) -> PlatformOutbox: async def send( chat_id: str, text: str, @@ -23,18 +23,14 @@ def _noop_outbox(*, limiter=None) -> PlatformOutbox: ) -> None: return None - async def delete(chat_id: str, message_id: str) -> None: - return None - - async def delete_many(chat_id: str, message_ids: list[str]) -> None: + async def default_delete_many(chat_id: str, message_ids: list[str]) -> None: return None return PlatformOutbox( get_limiter=lambda: limiter, send=send, edit=edit, - delete=delete, - delete_many=delete_many, + delete_many=delete_many or default_delete_many, ) @@ -74,17 +70,6 @@ async def test_queue_edit_awaits_limiter_with_dedup_key() -> None: await operation() -@pytest.mark.asyncio -async def test_queue_delete_uses_fire_and_forget_with_dedup_key() -> None: - limiter = MagicMock() - outbox = _noop_outbox(limiter=limiter) - - await outbox.queue_delete_message("chat", "message", fire_and_forget=True) - - limiter.fire_and_forget.assert_called_once() - assert limiter.fire_and_forget.call_args.kwargs["dedup_key"] == "del:chat:message" - - @pytest.mark.asyncio async def test_queue_delete_many_skips_empty_batches() -> None: limiter = MagicMock() @@ -103,6 +88,26 @@ async def test_queue_delete_many_dedups_by_batch() -> None: await outbox.queue_delete_messages("chat", ["1", "2"], fire_and_forget=True) limiter.fire_and_forget.assert_called_once() - assert limiter.fire_and_forget.call_args.kwargs["dedup_key"] == ( - f"del_bulk:chat:{hash(('1', '2'))}" + assert ( + limiter.fire_and_forget.call_args.kwargs["dedup_key"] + == "del_bulk:chat:11f0530a8259fffb" ) + + +@pytest.mark.asyncio +async def test_queue_delete_many_snapshots_ids_before_queueing() -> None: + limiter = MagicMock() + deleted: list[list[str]] = [] + + async def delete_many(_chat_id: str, message_ids: list[str]) -> None: + deleted.append(message_ids) + + outbox = _noop_outbox(limiter=limiter, delete_many=delete_many) + message_ids = ["1", "2"] + + await outbox.queue_delete_messages("chat", message_ids, fire_and_forget=True) + message_ids.append("3") + operation = limiter.fire_and_forget.call_args.args[0] + await operation() + + assert deleted == [["1", "2"]] diff --git a/tests/messaging/test_platform_voice_flow.py b/tests/messaging/test_platform_voice_flow.py index 8abfab13..0b81cb1e 100644 --- a/tests/messaging/test_platform_voice_flow.py +++ b/tests/messaging/test_platform_voice_flow.py @@ -69,7 +69,7 @@ async def test_voice_flow_success_builds_incoming_message(monkeypatch) -> None: _request(download_to=download_to), message_handler=handler, queue_send_message=queue_send, - queue_delete_message=queue_delete, + queue_delete_messages=queue_delete, ) assert handled is True @@ -105,7 +105,7 @@ async def test_voice_flow_disabled_replies_without_transcribing(monkeypatch) -> _request(reply_text=reply_text), message_handler=AsyncMock(), queue_send_message=AsyncMock(), - queue_delete_message=AsyncMock(), + queue_delete_messages=AsyncMock(), ) assert handled is True @@ -134,12 +134,12 @@ async def test_voice_flow_cancelled_transcription_deletes_status(monkeypatch) -> _request(), message_handler=handler, queue_send_message=queue_send, - queue_delete_message=queue_delete, + queue_delete_messages=queue_delete, ) assert handled is True handler.assert_not_awaited() - queue_delete.assert_awaited_once_with("chat", "status") + queue_delete.assert_awaited_once_with("chat", ["status"]) @pytest.mark.asyncio @@ -157,12 +157,12 @@ async def test_voice_flow_download_failure_cleans_pending_state(monkeypatch) -> _request(download_to=failing_download, reply_text=reply_text), message_handler=AsyncMock(), queue_send_message=AsyncMock(return_value="status"), - queue_delete_message=queue_delete, + queue_delete_messages=queue_delete, ) assert handled is True transcribe.assert_not_awaited() - queue_delete.assert_awaited_once_with("chat", "status") + queue_delete.assert_awaited_once_with("chat", ["status"]) reply_text.assert_awaited_once_with(VOICE_TRANSCRIPTION_ERROR_MESSAGE) assert await flow.cancel_pending_voice("chat", "voice") is None @@ -184,11 +184,11 @@ async def test_voice_flow_transcription_failure_cleans_pending_state( _request(reply_text=reply_text), message_handler=AsyncMock(), queue_send_message=AsyncMock(return_value="status"), - queue_delete_message=queue_delete, + queue_delete_messages=queue_delete, ) assert handled is True - queue_delete.assert_awaited_once_with("chat", "status") + queue_delete.assert_awaited_once_with("chat", ["status"]) reply_text.assert_awaited_once_with(VOICE_TRANSCRIPTION_ERROR_MESSAGE) assert await flow.cancel_pending_voice("chat", "voice") is None @@ -213,7 +213,7 @@ async def test_voice_flow_handler_failure_cleans_pending_without_deleting_status _request(reply_text=reply_text), message_handler=failing_handler, queue_send_message=AsyncMock(return_value="status"), - queue_delete_message=queue_delete, + queue_delete_messages=queue_delete, ) assert handled is True diff --git a/tests/messaging/test_telegram.py b/tests/messaging/test_telegram.py index 8db2eb91..bc211de5 100644 --- a/tests/messaging/test_telegram.py +++ b/tests/messaging/test_telegram.py @@ -1,6 +1,7 @@ from unittest.mock import AsyncMock, MagicMock, patch import pytest +from telegram.error import TelegramError from messaging.platforms.telegram import TelegramRuntime @@ -113,6 +114,132 @@ async def test_telegram_platform_edit_message_success(telegram_platform): ) +@pytest.mark.asyncio +async def test_telegram_platform_delete_messages_uses_batch_api(telegram_platform): + mock_bot = AsyncMock() + telegram_platform._application = MagicMock() + telegram_platform._application.bot = mock_bot + + await telegram_platform.outbound.delete_messages("chat_1", ["1", "2", "bad"]) + + mock_bot.delete_messages.assert_awaited_once_with( + chat_id="chat_1", + message_ids=[1, 2], + ) + mock_bot.delete_message.assert_not_awaited() + + +@pytest.mark.asyncio +async def test_telegram_platform_delete_messages_chunks_batch_api(telegram_platform): + mock_bot = AsyncMock() + telegram_platform._application = MagicMock() + telegram_platform._application.bot = mock_bot + + await telegram_platform.outbound.delete_messages( + "chat_1", + [str(i) for i in range(105)], + ) + + assert mock_bot.delete_messages.await_count == 2 + assert mock_bot.delete_messages.await_args_list[0].kwargs["message_ids"] == list( + range(100) + ) + assert mock_bot.delete_messages.await_args_list[1].kwargs["message_ids"] == list( + range(100, 105) + ) + + +@pytest.mark.asyncio +async def test_telegram_platform_delete_messages_falls_back_without_batch( + telegram_platform, +): + class BotWithoutBatch: + def __init__(self) -> None: + self.delete_message = AsyncMock() + + bot = BotWithoutBatch() + telegram_platform._application = MagicMock() + telegram_platform._application.bot = bot + + await telegram_platform.outbound.delete_messages("chat_1", ["1", "2"]) + + assert bot.delete_message.await_args_list[0].kwargs == { + "chat_id": "chat_1", + "message_id": 1, + } + assert bot.delete_message.await_args_list[1].kwargs == { + "chat_id": "chat_1", + "message_id": 2, + } + + +@pytest.mark.asyncio +async def test_telegram_platform_delete_messages_falls_back_after_batch_failure( + telegram_platform, +): + mock_bot = AsyncMock() + mock_bot.delete_messages.side_effect = RuntimeError("bulk failed") + telegram_platform._application = MagicMock() + telegram_platform._application.bot = mock_bot + + await telegram_platform.outbound.delete_messages("chat_1", ["1", "2"]) + + mock_bot.delete_messages.assert_awaited_once_with( + chat_id="chat_1", + message_ids=[1, 2], + ) + assert mock_bot.delete_message.await_args_list[0].kwargs == { + "chat_id": "chat_1", + "message_id": 1, + } + assert mock_bot.delete_message.await_args_list[1].kwargs == { + "chat_id": "chat_1", + "message_id": 2, + } + + +@pytest.mark.asyncio +async def test_telegram_platform_delete_messages_falls_back_after_swallowed_error( + telegram_platform, +): + mock_bot = AsyncMock() + mock_bot.delete_messages.side_effect = TelegramError("message can't be deleted") + telegram_platform._application = MagicMock() + telegram_platform._application.bot = mock_bot + + await telegram_platform.outbound.delete_messages("chat_1", ["1", "2"]) + + mock_bot.delete_messages.assert_awaited_once_with( + chat_id="chat_1", + message_ids=[1, 2], + ) + assert mock_bot.delete_message.await_args_list[0].kwargs == { + "chat_id": "chat_1", + "message_id": 1, + } + assert mock_bot.delete_message.await_args_list[1].kwargs == { + "chat_id": "chat_1", + "message_id": 2, + } + + +@pytest.mark.asyncio +async def test_telegram_platform_single_delete_still_swallows_known_error( + telegram_platform, +): + mock_bot = AsyncMock() + mock_bot.delete_message.side_effect = TelegramError("message can't be deleted") + telegram_platform._application = MagicMock() + telegram_platform._application.bot = mock_bot + + await telegram_platform.outbound.delete_message("chat_1", "1") + + mock_bot.delete_message.assert_awaited_once_with( + chat_id="chat_1", + message_id=1, + ) + + @pytest.mark.asyncio async def test_telegram_platform_queue_send_message(telegram_platform): mock_limiter = AsyncMock() diff --git a/tests/messaging/test_telegram_edge_cases.py b/tests/messaging/test_telegram_edge_cases.py index 1351b4cc..cd15b9c3 100644 --- a/tests/messaging/test_telegram_edge_cases.py +++ b/tests/messaging/test_telegram_edge_cases.py @@ -113,6 +113,23 @@ async def test_with_retry_drops_parse_mode_on_markdown_entity_error(): assert calls == ["MarkdownV2", None] +@pytest.mark.asyncio +async def test_with_retry_can_raise_known_message_errors_for_bulk_fallback(): + with patch("messaging.platforms.telegram.TELEGRAM_AVAILABLE", True): + from messaging.platforms.telegram import TelegramRuntime + + platform = TelegramRuntime(bot_token="t") + + async def _f(): + raise TelegramError("message can't be deleted") + + with pytest.raises(TelegramError): + await platform.outbound._with_retry( + _f, + suppress_known_message_errors=False, + ) + + @pytest.mark.asyncio async def test_queue_send_message_without_limiter_calls_send_message(): with patch("messaging.platforms.telegram.TELEGRAM_AVAILABLE", True): diff --git a/uv.lock b/uv.lock index 9d676953..21eb51a9 100644 --- a/uv.lock +++ b/uv.lock @@ -561,7 +561,7 @@ wheels = [ [[package]] name = "free-claude-code" -version = "3.4.1" +version = "3.4.2" source = { editable = "." } dependencies = [ { name = "aiohttp" },