Skip to content

fix(telegram): cache channel-post media via effective_message (#51543) - #51747

Open
michaelversluis wants to merge 1 commit into
NousResearch:mainfrom
michaelversluis:fix/telegram-channel-media-51543
Open

fix(telegram): cache channel-post media via effective_message (#51543)#51747
michaelversluis wants to merge 1 commit into
NousResearch:mainfrom
michaelversluis:fix/telegram-channel-media-51543

Conversation

@michaelversluis

Copy link
Copy Markdown

Fixes #51543

Problem

Telegram delivers channel broadcasts as update.channel_post, not update.message. _handle_media_message guarded on if not update.message: and bailed immediately for channel posts, so file / photo / video / voice attachments sent to a broadcast channel were silently dropped — only the caption text reached the agent and nothing was cached at ~/.hermes/cache/.

The sibling handlers _handle_text_message and _handle_command already resolve the payload via _effective_update_message(update) (which returns effective_message → message → None). The media handler was missed during the plugin refactor that deleted the old gateway/platforms/telegram.py (commit 560010547); this ports the same fix to the new plugin location.

Fix

  • Resolve the message via _effective_update_message(update) and replace the five update.message references in the early-return block with the resolved msg, mirroring the text/command handlers. The rest of the method already used a local msg.

Tests

  • New regression test test_media_handler_uses_effective_message_for_channel_post in tests/gateway/test_telegram_channel_posts.py: sends a channel-post photo and asserts it is routed into the caching pipeline. Verified RED on the unpatched handler (the early return means _enqueue_photo_event is never called) and GREEN after the fix.
  • Updated _make_update in tests/gateway/test_telegram_documents.py to expose effective_message alongside message, matching real python-telegram-bot Update objects (where effective_message always resolves the active payload). Without this the existing mock only modelled .message.
  • Full media regression suite green: test_telegram_channel_posts.py, test_telegram_documents.py, test_telegram_group_gating.py → 90 passed.

NL — kort voor de eigen administratie: kanaal-broadcasts kwamen binnen als channel_post i.p.v. message, waardoor de media-handler meteen afhaakte en bijlagen in kanalen nooit werden gecached. Dezelfde effective_message-resolutie die de tekst- en command-handlers al gebruiken nu ook in de media-handler gezet, met een regressietest die zonder de fix faalt.

@alt-glitch alt-glitch added type/bug Something isn't working comp/gateway Gateway runner, session dispatch, delivery platform/telegram Telegram bot adapter P2 Medium — degraded but workaround exists sweeper:risk-message-delivery Sweeper risk: may drop, duplicate, misroute, or suppress messages labels Jun 24, 2026
@alt-glitch

Copy link
Copy Markdown
Collaborator

This was generated by AI during triage.

Related: this is the same channel_post media fix as #28614, #28813, and #35720 (all open), plus the closed #29753 — all route _handle_media_message through _effective_update_message(update) instead of update.message.

Key difference for the maintainer: the Telegram adapter was relocated during the plugin refactor. The three earlier open PRs target the old path gateway/platforms/telegram.py, which no longer exists on main; this PR targets the current location plugins/platforms/telegram/adapter.py, so it is the implementable version against current main. Flagging the cluster so a human can pick which to merge and close the stale ones.

@teknium1 teknium1 left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for targeting the current plugin adapter and adding a focused channel-post media regression. The reported early return still exists on current main at plugins/platforms/telegram/adapter.py:7764.

Problems

  • Current main added the intake authorization prefilter in commit c648ecdca after this PR's base. A salvage must also replace the later update.message references in the auth block at plugins/platforms/telegram/adapter.py:7766-7770 with the resolved msg. Otherwise channel posts pass None into _is_user_authorized_from_message(); that helper derives a channel identity from message.sender_chat (adapter.py:769-827), so authorization would not inspect the actual broadcast sender.

Suggested changes

  • Resolve msg before every media gate and use it for authorization, logging, group observation, and processing.
  • Add a channel-post media test with a non-allowlisted sender_chat that asserts no cache/enqueue occurs, alongside the positive caching test.

Automated hermes-sweeper review.

# matching ``_handle_text_message`` (which was already fixed). Guarding on
# ``update.message`` alone silently dropped every channel-post attachment.
msg = self._effective_update_message(update)
if not msg:

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

When salvaging this against current main, also pass msg to the authorization prefilter added by c648ecdca after this PR's base. Leaving that call on update.message gives channel posts None, so the prefilter cannot authorize their sender_chat identity.

@teknium1 teknium1 added sweeper:risk-security-boundary Sweeper risk: may affect sandboxing, auth, credentials, or sensitive data sweeper:blast-moderate Sweeper blast radius: moderate — a subsystem or single platform labels Jul 15, 2026
@michaelversluis
michaelversluis force-pushed the fix/telegram-channel-media-51543 branch from 0537809 to c4fd410 Compare July 15, 2026 03:25
@michaelversluis

Copy link
Copy Markdown
Author

Rebased onto current main (6997dc8) and addressed the sweeper findings:

  • The intake authorization prefilter added in c648ecd now receives the resolved msg instead of update.message, so channel broadcasts are authorized against their sender_chat identity (via _source_from_message_for_auth) rather than passing None. The unauthorized-log lines use msg as well.
  • Added the requested negative test: test_media_handler_blocks_unauthorized_channel_post — a channel-post photo whose sender_chat is not on the adapter allow_from allowlist is rejected at intake with no caching, no enqueue, and no file download. Verified RED on the unpatched auth call (passing update.message) and GREEN with the fix.
  • Full telegram gateway test neighbourhood run: the channel-post suite (5 tests) and document suite pass; the handful of unrelated failures (network-reconnect classifier, slash-confirm markdown) reproduce identically on clean main and are pre-existing.

PR is mergeable again.

@michaelversluis

Copy link
Copy Markdown
Author

@teknium1 good like this?

@michaelversluis

Copy link
Copy Markdown
Author

@alt-glitch

…search#51543)

Telegram delivers channel broadcasts as `update.channel_post`, not
`update.message`. `_handle_media_message` guarded on `update.message`
alone and bailed immediately for channel posts, so file/photo/video/voice
attachments to a broadcast channel were silently dropped — only the
caption text reached the agent and nothing was cached.

`_handle_text_message` and `_handle_command` already resolve the payload
via `_effective_update_message(update)` (effective_message → message →
None); the media handler was missed during the plugin refactor that
deleted the old `gateway/platforms/telegram.py` (commit 5600105).

- Resolve the message via `_effective_update_message` and replace the
  five `update.message` references in the early-return block with the
  resolved `msg`, mirroring the text/command handlers.
- Add a regression test asserting a channel-post photo is routed into the
  caching pipeline (RED before the fix: the handler returns early and
  never enqueues the photo).
- Update `_make_update` in test_telegram_documents.py to expose
  `effective_message`, matching real python-telegram-bot Update objects.

Fixes NousResearch#51543
@michaelversluis
michaelversluis force-pushed the fix/telegram-channel-media-51543 branch from c4fd410 to b53228e Compare August 10, 2026 04:08
@michaelversluis

Copy link
Copy Markdown
Author

Rebased onto current main (8359e760) and resolved the channel-post test conflict mechanically. The authorization regression now uses the current channel/group allowlist contract (group_allow_from). Verified: 20/20 focused Telegram tests pass; Ruff and git diff --check are clean. The PR is mergeable again.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

comp/gateway Gateway runner, session dispatch, delivery P2 Medium — degraded but workaround exists platform/telegram Telegram bot adapter sweeper:blast-moderate Sweeper blast radius: moderate — a subsystem or single platform sweeper:risk-message-delivery Sweeper risk: may drop, duplicate, misroute, or suppress messages sweeper:risk-security-boundary Sweeper risk: may affect sandboxing, auth, credentials, or sensitive data type/bug Something isn't working

Projects

None yet

Development

Successfully merging this pull request may close these issues.

# Bug Report: Telegram channel file attachments not delivered

3 participants