Skip to content

fix(telegram): discard buffered ingress fragments at conversation boundaries - #81528

Closed
Enough1122 wants to merge 2 commits into
NousResearch:mainfrom
Enough1122:fix/81370-telegram-attachment-buffer-leak
Closed

fix(telegram): discard buffered ingress fragments at conversation boundaries#81528
Enough1122 wants to merge 2 commits into
NousResearch:mainfrom
Enough1122:fix/81370-telegram-attachment-buffer-leak

Conversation

@Enough1122

Copy link
Copy Markdown
Contributor

Fixes #81370

Root cause

Telegram keeps per-session debounce slots for inbound text batching, photo bursts/albums, and media-group coalescing (implemented for #46100 / #46101 in plugins/platforms/telegram/adapter.py):

  • _pending_text_batches / _pending_text_batch_tasks_enqueue_text_event
  • _pending_photo_batches / _pending_photo_batch_tasks_flush_photo_batch
  • _media_group_events / _media_group_tasks_queue_media_group_event

Only disconnect() (_cancel_pending_delayed_deliveries) cleared them. A conversation boundary (/stop, /new, /reset, auto-reset, expiry finalization) never triggers disconnect(), so a forward or media download that finished after the reset kept its fragments in those slots — and the next unrelated user message merged them.

The production evidence from the issue: forwarded video 411357 was still in the startup/photo slot after /stop (411367); 2+ minutes later, ordinary copied text 411376 merged it and the gateway logged Merged 1 Telegram startup attachment(s) for a turn where the user sent no media.

Fix

  1. New hook BasePlatformAdapter._discard_session_ingress(session_key) in gateway/platforms/base.py (default no-op), funneled through _discard_text_debounce, which every conversation boundary already calls via cancel_session_processing and the /stop,/new,/reset bypass-dispatch path.
  2. TelegramAdapter._discard_session_ingress overrides it: drops the session's text-batch, photo-burst/album, and media-group slots and cancels their flush tasks. Unrelated sessions' buffers are keyed differently and survive untouched.

The fix is session-scoped — one conversation's reset cannot affect a concurrent session's in-flight batch.

Regression tests

5 tests in tests/gateway/test_telegram_ingress_boundary_81370.py:

  1. test_text_batch_for_session_is_dropped — the issue's scenario: a stale text-batch fragment is gone after the boundary.
  2. test_other_sessions_batches_survive — session scoping: another session's batch is untouched.
  3. test_photo_burst_and_album_slots_for_session_are_dropped — photo burst (session:photo-burst) and album (session:album:<id>) slots both cleared.
  4. test_media_group_slots_for_session_are_dropped — media-group coalescing keyed by media_group_id is matched via the buffered event's source.
  5. test_base_hook_funnel_discard_text_debounce funnels into the adapter override, so every existing boundary that clears the base debounce also clears Telegram's slots.

All 5 are RED on pre-fix code (hook missing → AttributeError) and GREEN after. Existing Telegram tests (test_telegram_caption_merge.py, test_telegram_auth_check.py, test_telegram_audio_vs_voice.py, test_telegram_approval_buttons.py — 27 tests) continue to pass.

Scope

One new base hook (no-op default) + one adapter override + tests. No refactor of the debounce machinery itself; the existing boundary funnel is reused rather than adding a parallel cleanup path.

@alt-glitch alt-glitch added type/bug Something isn't working comp/gateway Gateway runner, session dispatch, delivery comp/cli CLI entry point, hermes_cli/, setup wizard comp/plugins Plugin system and bundled plugins platform/telegram Telegram bot adapter P2 Medium — degraded but workaround exists sweeper:risk-session-state Sweeper risk: may lose/corrupt/mis-associate session or context state sweeper:risk-message-delivery Sweeper risk: may drop, duplicate, misroute, or suppress messages labels Aug 8, 2026
@alt-glitch

Copy link
Copy Markdown
Collaborator

This was generated by AI during triage.

Related to #81371, which addresses #81370 with a broader generation-scoped ingress design. This PR uses explicit session-boundary cancellation and also includes an unrelated oneshot fallback change for #81209; please consolidate the overlapping Telegram fix before merge.

@Enough1122
Enough1122 force-pushed the fix/81370-telegram-attachment-buffer-leak branch from 9d6e950 to c0e0bc1 Compare August 8, 2026 05:52
@Enough1122

Copy link
Copy Markdown
Contributor Author

Critical fix: the media-group branch of _discard_session_ingress matched event.source.session_key == session_key, but SessionSource has no session_key attribute — the session key is always derived via build_session_key(source). The lookup returned None for every real source, so the media-group cleanup branch was dead code and Telegram media-group albums were never invalidated at conversation boundaries (the exact scenario the PR title targets). The existing test masked the defect with a fabricated source fixture.

This follow-up:

  • Recomputes the key via build_session_key(event.source, ...) (with try/except) so the cleanup matches real SessionSource objects.
  • Rewrites the regression test to use a real SessionSource (Platform.TELEGRAM) so this branch can never silently regress.

5/5 tests pass locally.

@Enough1122

Copy link
Copy Markdown
Contributor Author

Closing in favor of #81371 (Qwinty), which was opened ~6h earlier (2026-08-07T23:20Z vs 08-08T05:24Z) and fixes the same issue #81370 (stale Telegram ingress fragments leaking across conversation boundaries) in the same files (gateway/platforms/base.py + plugins/platforms/telegram/adapter.py). Our 5 regression tests in tests/gateway/test_telegram_ingress_boundary_81370.py remain on branch fix/81370-telegram-attachment-buffer-leak if maintainers want to salvage any of them.

@Enough1122 Enough1122 closed this Aug 12, 2026
@Enough1122 Enough1122 reopened this Aug 12, 2026
…ndaries

Telegram keeps per-session debounce slots for text batching, photo
bursts/albums, and media-group coalescing (implemented for NousResearch#46100 /
download that finished after the conversation was reset (/stop, /new,
/reset, auto-reset, expiry finalization) kept its fragments in those
slots, and the next unrelated user message merged them into its turn.

Production evidence from NousResearch#81370: a forwarded video was still in a
debounce slot after /stop; two minutes later an ordinary copied-text
message merged it, and the gateway logged 'Merged 1 Telegram startup
attachment(s)' for a turn that carried no media from the user.

Fix: add a session-scoped ingress-discard hook
(BasePlatformAdapter._discard_session_ingress) and funnel it through
the existing _discard_text_debounce path, which every conversation
boundary already calls via cancel_session_processing.  TelegramAdapter
overrides it to drop text/photo/media-group buffers keyed to that
session and cancel their flush tasks.  Unrelated sessions' buffers are
untouched.

Adds 5 regression tests covering: text-batch drop, other-session
survival, photo burst+album drop, media-group drop, and the base-hook
funnel.  All RED on pre-fix code (hook missing), GREEN after.
…ale attribute (NousResearch#81370)

The media-group branch of `_discard_session_ingress` matched
`event.source.session_key == session_key`, but `SessionSource` has
no `session_key` attribute — the session key is always *derived* via
`build_session_key(source)`. The lookup was always None, so the
media-group cleanup branch was dead code and albums were never
invalidated at conversation boundaries. The existing test masked the
defect with a fabricated source fixture.

- Recompute the key with `build_session_key` so real `SessionSource`
  objects match correctly.
- Rewrite the regression test to use a real `SessionSource` (with
  `Platform.TELEGRAM`) so this branch can never silently regress.
@Enough1122
Enough1122 force-pushed the fix/81370-telegram-attachment-buffer-leak branch from 077d7e9 to e592c50 Compare August 12, 2026 05:38
@Enough1122 Enough1122 closed this Aug 13, 2026
dvbaecker added a commit to dvbaecker/hermes-agent that referenced this pull request Aug 13, 2026
…troying them

The disconnect drop-guard (NousResearch#55971) correctly prevents dispatch into a
torn-down session. Destroying the event was wrong: by enqueue/flush time
python-telegram-bot has already acked the update and advanced the polling
offset, so Telegram never redelivers. Result: silent permanent loss, no
log, no error.

Hold inbound events (text/photo/media-group) when the drop-guard fires,
salvage pending batch maps on teardown, cancel+await the redispatch task
in the delivery cancel map (lifecycle-tracked), and redispatch from
_mark_connected after reconnect. Cap the hold queue (default 64), dedupe
by object identity, discard on non-retryable fatal. Cancel-after-pop in
flush paths also holds.

Distinct from NousResearch#72037 (cancel-after-pop during follow-up supersession) and
NousResearch#81528 (boundary discard). Tests use delay=0 and entered/release Events —
no wall-clock races; includes production terminal-step coverage.
kshitijk4poor pushed a commit that referenced this pull request Aug 15, 2026
…troying them

The disconnect drop-guard (#55971) correctly prevents dispatch into a
torn-down session. Destroying the event was wrong: by enqueue/flush time
python-telegram-bot has already acked the update and advanced the polling
offset, so Telegram never redelivers. Result: silent permanent loss, no
log, no error.

Hold inbound events (text/photo/media-group) when the drop-guard fires,
salvage pending batch maps on teardown, cancel+await the redispatch task
in the delivery cancel map (lifecycle-tracked), and redispatch from
_mark_connected after reconnect. Cap the hold queue (default 64), dedupe
by object identity, discard on non-retryable fatal. Cancel-after-pop in
flush paths also holds.

Distinct from #72037 (cancel-after-pop during follow-up supersession) and
#81528 (boundary discard). Tests use delay=0 and entered/release Events —
no wall-clock races; includes production terminal-step coverage.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

comp/cli CLI entry point, hermes_cli/, setup wizard comp/gateway Gateway runner, session dispatch, delivery comp/plugins Plugin system and bundled plugins P2 Medium — degraded but workaround exists platform/telegram Telegram bot adapter sweeper:risk-message-delivery Sweeper risk: may drop, duplicate, misroute, or suppress messages sweeper:risk-session-state Sweeper risk: may lose/corrupt/mis-associate session or context state type/bug Something isn't working

Projects

None yet

Development

Successfully merging this pull request may close these issues.

fix(telegram): forwarded attachment buffers survive /stop and merge into the next unrelated turn

2 participants