fix(telegram): address 5 post-merge audit follow-ups#28681
Merged
Conversation
…28674, #28676, #28678) Five small fixes against issues filed during the post-merge salvage audit: * #28670: `_GATEWAY_PROVIDER_ERROR_RE` false-positives on legitimate prose. Replace the regex with an anchored `_GATEWAY_PROVIDER_ERROR_SHAPE_RE` and add a length-cap heuristic to `_looks_like_gateway_provider_error`: short envelope at the start of the message → real provider error; long prose containing 'HTTP 404' → assistant answer, leave alone. * #28672: drop the pointless 1s asyncio.sleep on Telegram thread-not-found retries. The same-thread retry is preserved (catches Telegram's occasional transient flake exercised by test_send_retries_transient_thread_not_found_before_fallback) but with no artificial delay. * #28674: broaden `_should_retry_without_dm_topic_reply_anchor` to also fire when Bot API rejects `direct_messages_topic_id` for synthetic / resumed sends that have no reply anchor. Avoids dropping post-resume background notifications if the topic id goes stale. * #28676: delete the dead image-document branch superseded by bd0c54d (which returns early on the same extension set). * #28678: extend chat-scoped allowlist (`TELEGRAM_GROUP_ALLOWED_CHATS`) to also cover `chat_type == 'channel'`, so operators can authorize channel posts by chat id without falling back to per-user allowlists. Tests: - scripts/run_tests.sh tests/gateway/test_telegram_thread_fallback.py -q → 41/41 - scripts/run_tests.sh tests/cron/test_scheduler.py -q → 127/127 - broader test set: same 3 pre-existing test-pollution failures reproduce on plain main.
Contributor
🔎 Lint report:
|
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Fixes 5 small issues filed during the post-merge salvage audit. Single PR because each fix is small and they touch related files.
Resolves
_GATEWAY_PROVIDER_ERROR_REfalse-positives on legitimate prosedirect_messages_topic_idhad no retry pathChanges
#28670 — anchor + length-cap the provider-error sanitizer
_looks_like_gateway_provider_errornow uses an anchored regex (^\s*(\W*\s*)?...) and refuses to rewrite messages over 400 chars or 4+ lines. A user asking 'what does HTTP 404 mean?' on Telegram no longer has their entire reply replaced with the provider-error template; the rewrite still fires on actual short provider error envelopes that lead with 'HTTP 503' / 'API call failed' / etc.#28672 — drop the 1s sleep, keep the retry
The original code did
asyncio.sleep(1)then retried with the samemessage_thread_id. The sleep added latency on every thread-not-found error; the retry IS sometimes useful (Telegram has a one-off thread-not-found flake mode exercised bytest_send_retries_transient_thread_not_found_before_fallback), so I kept the retry but removed the sleep.#28674 — extend DM-topic retry predicate
_should_retry_without_dm_topic_reply_anchorpreviously requiredreply_to_message_id is not None, so synthetic / resumed sends that route viadirect_messages_topic_id(no anchor) had no retry path if Bot API rejected the topic id. Predicate now also fires whendirect_messages_topic_idis set and the BadRequest mentions a topic/thread routing failure. The retry path already correctly strips both fields together — only the trigger needed widening.#28676 — remove dead branch
Lines 4947-4960 of
gateway/platforms/telegram.pycheckedext in SUPPORTED_IMAGE_DOCUMENT_TYPESfor.png/.jpg/.jpeg/.webp/.gif. The earlier branch at line 4896 (bd0c54d17 fix: route Telegram image documents through photo handling) already handles the exact same extension set andreturns before reaching here. Replaced the dead block with a comment.#28678 — chat-scoped auth covers channels
source.chat_type in {'group', 'forum'}extended to{'group', 'forum', 'channel'}for the chat-scoped allowlist in_is_user_authorized. Operators can now put a channel id inTELEGRAM_GROUP_ALLOWED_CHATSand channel posts get authorized correctly. Previously the only paths that worked for channels were either listing the channel id inTELEGRAM_ALLOWED_USERS(because_build_message_eventsynthesizesuser_id = chat.idfor channel posts) orGATEWAY_ALLOW_ALL_USERS=true.Validation
The 3 failures in the broader set are pre-existing test-pollution failures that reproduce on plain main without these changes.