← Back to PRs

#15985: fix(telegram): defer buffer deletion until processing succeeds

by coygeek open 2026-02-14 04:26 View on GitHub →
channel: telegram stale size: M trusted-contributor
## Fix Summary Defer buffer entry deletion in Telegram text-fragment and media-group pipelines until processing completes successfully. On transient failure, reschedule the flush instead of silently dropping messages. ## Issue Linkage Fixes #15984 ## Security Snapshot - CVSS v3.1: 8.2 (High) - CVSS v4.0: 8.8 (High) ## Implementation Details ### Files Changed - `src/telegram/bot-handlers.ts` (+98/-34) — core fix: queue helpers with flushing guard, deferred deletion, retry-on-failure - `src/telegram/bot-updates.ts` (+1) — add `flushing` field to `MediaGroupEntry` type - `src/telegram/bot.media.downloads-media-file-path-no-file-download.e2e.test.ts` (+382) — regression tests for buffered flush behavior ### Technical Analysis Both `textFragmentBuffer` and `mediaGroupBuffer` previously called `.delete()` before invoking `processMessage`/`processMediaGroup`. A transient failure in processing meant the entry was already gone — no retry, no user signal, permanent silent loss. The fix introduces: 1. A `flushing` boolean guard on buffer entries to prevent concurrent modification 2. `queueTextFragmentFlush` / `queueMediaGroupFlush` helpers that only delete after success 3. On failure, `flushing` is reset and the entry is rescheduled via the existing timer mechanism ## Validation Evidence - Command: `pnpm build && pnpm check` - Status: passed - Command: `pnpm vitest run src/telegram/` - Status: 426 tests passed (50 test files) ## Risk and Compatibility Non-breaking. Failed flushes now retry instead of silently dropping. Successful processing behavior is unchanged. ## AI-Assisted Disclosure - AI-assisted: yes - Model: Claude Opus 4.6 <!-- greptile_comment --> <h2>Greptile Overview</h2> <h3>Greptile Summary</h3> Defers buffer deletion until message processing succeeds, preventing silent message loss on transient failures. **Key changes:** - Added `flushing` boolean guard to `TextFragmentEntry` and `MediaGroupEntry` to prevent concurrent buffer modifications - Created `queueTextFragmentFlush` and `queueMediaGroupFlush` wrappers that set the guard, attempt processing, and only delete on success - Modified `processMediaGroup` and `flushTextFragments` to return boolean success indicators - On failure, the `flushing` flag is cleared and processing is rescheduled via existing timer mechanism - Updated buffer append logic to check `!existing.flushing` before appending, creating new entries for messages arriving during flush **Behavior:** - Previously: buffer entry deleted before processing, so failures meant permanent message loss - Now: deletion deferred until after successful processing, failures trigger retry via rescheduling - Messages arriving during flush are buffered in a new entry (tested in "keeps late media/text items" tests) - Identity checks (`mediaGroupBuffer.get(mediaGroupId) === entry`) prevent deleting wrong entries after buffer replacement <h3>Confidence Score: 5/5</h3> - This PR is safe to merge with minimal risk - The implementation correctly fixes the data loss vulnerability without introducing new issues. The flushing guard prevents race conditions, identity checks prevent wrong-entry deletion, and comprehensive test coverage validates retry behavior and concurrent message handling. The fix is backwards-compatible (successful paths unchanged) and follows existing patterns in the codebase. - No files require special attention <sub>Last reviewed commit: 21178ac</sub> <!-- greptile_other_comments_section --> <!-- /greptile_comment -->

Most Similar PRs