feat(telegram): await polled handlers before advancing offset; combine incoming albums (#227)

[patrick-chinchill]

Summary

Telegram long polling used to advance offset before any handler ran, so a handler failure or a crash lost the update. Now it works like upstream 4.41:

• Polling waits for handlers. The poller awaits every handler task of a batch before offset moves past it. Failed updates are saved in a telegram:polling:{sha256(bot_user_id)} checkpoint ({offset, pending: [{update, receivedAt, attempts, retryAt}]}) and retried with backoff. Polled updates are dispatched with WebhookOptions(deduplicate=False); the checkpoint deduplicates them, not core dedupe.
• Albums arrive as one message. Parts with a media_group_id are buffered in state (with a lock) and dispatched as one Message 1 s after the newest part. This works on both the webhook and polling paths. On polling, album parts wait in the checkpoint for the settle window and the album succeeds or fails as a unit.
• process_update returns the dispatched tasks: message, album, slash command, action and reactions.

Split: outbound multi-file / multi-attachment sendMediaGroup (8d7ccdb1, #605) is moved to #278. With it, this PR would have been about 1.9k LOC; the issue's Metadata says to split the outbound part off if it runs over. So this PR is Part of #227, not Closes.

Upstream commits mapped

Upstream	Where
629e6555 fix(telegram): combine incoming media groups (#760), chat@4.37.0	handle_incoming_message_update → _start_incoming_media_group / _process_incoming_media_group; slash gate skips album parts
91683e52 fix(telegram): wait for polling handlers before advancing offset (#942), chat@4.41.0, Telegram half	process_update returns tasks; polling_loop, _process_polling_updates, _polling_group, _fetch_polling_updates (the collecting abort timer), _await_update_tasks
26a06ca5 lazy identity retry	_ensure_bot_identity() runs on every loop iteration
8d7ccdb1 (#605)	not here, see #278

The core half of #942 (WebhookOptions.deduplicate, process_* returning tasks that raise) was already on main from #191. I checked this before starting.

Tests ported (tests/test_telegram_webhook.py)

From adapter-telegram/src/index.test.ts:

• combines an incoming media group into one ordered message (two adapters share one state)
• starts polling, advances offset, and stops cleanly
• waits for polled message processing and saves failures before acknowledging updates
• coalesces polled media groups before acknowledging updates

From integration-tests/src/polling.test.ts, run against a real Chat + MemoryStateAdapter and a fake Bot API that redelivers unacknowledged updates:

• saves a failed {message, command, action, reaction} before acknowledgement and keeps it until the retry succeeds
• recovers a saved failed update after process loss (with the core dedupe: key already set)
• does not acknowledge failed ordinary updates when saving their retry fails
• retries every member of a failed album
• polls between album batches when another album becomes ready during a handler
• retains the retry count and bounds album backoff (1 → 2 s, 8 → 30 s)
• keeps pending albums through cleanup failures without repeating successful handlers
• does not buffer albums from users outside the allowlist
• does not replay another bot's saved album
• rate-limited retry waits at least retry_after
• processes beyond a full page despite a permanently failing {message, command, action, reaction}
• combines an album spanning a full polling response
• recovers bot identity while polling after a failed startup getMe (from index.test.ts; asserts the checkpoint key, since mentionOnReply is [4.41/TG4] Telegram replies: replied-to context, reply-to-bot as mention, native replies, portable file data #228)

Python-specific:

• stop_polling during an album settle returns promptly with a consistent checkpoint, and a restart delivers the album once
• stop_polling waits for an in-flight handler
• a handler cancelled by Chat.shutdown keeps its update pending
• a failed album task is logged and wait_until settles, while the returned task still raises
• disconnect() cancelling a settling webhook album leaves the wait_until task settled (resolves to None), not cancelled
• album contract: newest-10 cap, a redelivered part is not duplicated, is_mention if any part mentions the bot, links concatenated, reply_to from the first part that has one

The tests use an injectable fake clock (_now_ms / _sleep) and never sleep on the wall clock. I also mutation-checked them: each of 20 mutations (the 12 below, plus the _settled cancel handling, reply_to, the per-iteration _ensure_bot_identity(), timeout=0 when a retry is due, the newest-10 cap, is_mention from any part, links, and the redelivery filter) (dropping the handler await, deduplicate=False, the album slash gate, the cancelled-handler failure, the completed set, unconditional cancel in stop_polling, retry_after, task collection for message/action/reaction, the allowlist group check, and drained) makes at least one test fail.

Fidelity

TS_ROOT=…/vercel-chat-4.41.1 verify_test_fidelity.py --report-target: Delta vs committed report (HEAD): missing 160 -> 160 (+0). adapter-telegram is not fidelity-mapped (#78), so scripts/fidelity_target.json is unchanged. Strict at the 4.31.0 pin passes ("All TS tests have Python equivalents").

Divergences

All three are recorded in the new docs/UPSTREAM_SYNC.md section "Telegram polling acknowledgement and incoming albums".

1. Stop semantics (an AbortController mapping, not a behavior change). stop_polling cancels the loop only while it is parked in getUpdates or a polling sleep, which are the awaits upstream's signal reaches. While handlers run or the checkpoint is written, it waits, as upstream stopPolling does. It waits with asyncio.wait, so cancelling the caller still propagates.
2. Cancelled handler = failure. Python Chat.shutdown cancels in-flight handler tasks (upstream waits for them). Without this, a cancelled handler would be acknowledged unhandled; now the update stays in the checkpoint.
3. Stop during the identity/checkpoint read. If stop_polling lands while the loop awaits _ensure_bot_identity() or the checkpoint state.get, the loop returns before dispatching the ready retry batch. Upstream pollingLoop (adapter-telegram/src/index.ts:4057-4088) has no pollingActive check there and still dispatches it; in Python that batch could start handlers after Chat.shutdown's cancellation sweep. The batch stays in the checkpoint for the next start.

reply_to on the combined album message is carried as upstream (index.ts:1295, first part that has one) now that Message.reply_to exists (#192); Telegram parsing populates it only from #228, so it is None at runtime today.

Clock: receivedAt / retryAt are epoch ms (as upstream Date.now()). They are persisted, so a monotonic clock would not survive a restart.

Astra review (gpt-6-astra)

Round 5: PASS ("No actionable regressions found").

Fixed:

• R1: the collection poll read fetch.result() before the cancelled fetch had finished, which raised InvalidStateError. Every album settle therefore went through the failure backoff. It now awaits the fetch first. Regression: the coalesce test asserts no "Telegram polling request failed" warning.
• R1: albums still settling escaped Chat.shutdown's handler cancellation. disconnect() now cancels _media_group_tasks before stopping the poller, and the album stays in the checkpoint. Regression: test_chat_shutdown_during_album_settle_does_not_wait_for_the_album.
• R3: stop_polling during the initial checkpoint read could still dispatch saved retries. The loop now rechecks _polling_active first. Regression: test_stop_during_the_checkpoint_read_dispatches_no_saved_retry.

Not changed, because the behavior is identical to upstream chat@4.41.1 (I added a code comment at each site):

• R1: the album buffer key is not bot-scoped (adapter-telegram/src/index.ts:1216).
• R2: with queue / debounce concurrency, a drained handler's failure is attributed to the update that drains the queue (chat.ts:2646-2729).
• R2: an album buffer that expires after a stall longer than 30 s returns without dispatch (index.ts:1248-1249).
• R2: a handler that awaits stop_polling deadlocks; upstream stopPolling awaits pollingTask (index.ts:846-847).
• R4: a handler dispatched after Chat.shutdown's Python-only cancellation sweep is admitted work that stop_polling waits for, as upstream does.

None of these was re-raised after the comment.

Consumer impact

Telegram only; none for Slack or Teams.

• Polling is now at-least-once with retries. Handlers can see the same update again after a failure or a crash, and a slow handler delays the next poll.
• Albums reach handlers as one message with N attachments, about 1 s later (about 2 s when polling), instead of N messages. An album caption that starts with /cmd is no longer routed as a slash command.
• process_update and the handle_* helpers now return tasks instead of None / bool.

Validation

ruff check/format, audit (0 hard failures), --check-docs, --strict (pin 4.31.0), pytest 6807 passed, 24 skipped (after merging origin/main at 98e0852), pyrefly 0 errors.

Part of #227
Part of #184

Merge gate

Final HEAD: fbd5f4b (merged origin/main at 98e0852; resolved the docs/UPSTREAM_SYNC.md conflicts twice (#192/#197 and #229) by keeping both sections; fidelity_target.json regenerated with no change).

Independent review findings (7):

• Fixed: wait_until album wrapper (_settled) ended cancelled when disconnect() cancelled a settling album. It now mirrors Chat._tracked._await_quietly: an album cancellation counts as settled, and only a cancellation of the wrapper itself propagates. Regression: test_waituntil_settles_when_disconnect_cancels_a_settling_album (fails without the fix with CancelledError). This was reported twice, as a parity finding and a robustness finding.
• Fixed: the combined album message now carries reply_to from the first part that has one, as upstream index.ts:1295 does, now that Message.reply_to exists ([4.41/C2a] Core mentions & message model: tri-state is_mention, mention regex, Author.email/is_system, Message.reply_to #192). The docs no longer say the field is missing. Regression: covered in the album contract test.
• Fixed (docs): a stop during the identity/checkpoint await skips the ready retry batch. This is now recorded as a Python-specific divergence in docs/UPSTREAM_SYNC.md, the CHANGELOG and the code comment (divergence 3 above). I kept the behavior.
• Fixed: the branch was behind main and had a docs conflict. I merged origin/main and kept both sides.
• Fixed (tests): the per-iteration _ensure_bot_identity() was untested. Added test_recovers_bot_identity_while_polling_after_a_failed_startup_get_me.
• Fixed (tests): five album/polling mutations survived (is_mention from any part, links, newest-10 cap, the redelivery filter, timeout=0 when due). Added the album contract test, a timeout == 0 assertion, and ports of "processes beyond a full page despite a permanently failing %s" and "combines an album spanning a full polling response". All 8 targeted mutations now fail the Telegram suite.
• Declined: none.

gpt-6-astra: 1 round on fbd5f4b. Verdict PASS: "No actionable regressions found relative to the supplied merge base. All 508 Telegram tests passed, and changed-file lint and formatting checks passed."

Bots: CodeRabbit skipped the draft and was then rate-limited ("Review limit reached"). There were no inline or review comments from CodeRabbit or gemini.

CI on fbd5f4b: Tests 3.12/3.13 pass, Lint & Type Check pass, CodeQL / Analyze pass.

Local: ruff check/format, audit (0 hard failures), --check-docs, --strict at the 4.31.0 pin, pytest 6807 passed / 24 skipped, pyrefly 0 errors.

[coderabbitai]

Warning

Review limit reached

You've used all free OSS reviews for now. Wait for the free limit to reset to keep reviewing this public repository.

Next included review available in 9 minutes.

Check out review usage here.

View limit details

Limit details: You’ve used the included review currently available.

Learn how review limits work.

Review configuration:

⚙️ Run configuration

Configuration used: Repository UI

Review profile: CHILL

Plan: Advanced

Run ID: 8b5097b8-eb9a-4543-bd13-2877c54571d1

📥 Commits

Reviewing files that changed from the base of the PR and between 570c95d and 1687ec0.

📒 Files selected for processing (5)

• CHANGELOG.md
• docs/UPSTREAM_SYNC.md
• src/chat_sdk/adapters/telegram/adapter.py
• src/chat_sdk/adapters/telegram/types.py
• tests/test_telegram_webhook.py

• Autopilot · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts

Autopilot is currently an internal CodeRabbit preview.

---

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

_{Comment @coderabbitai help to get the list of available commands.}

[patrick-chinchill]

Merge gate: CI green (test 3.12, test 3.13, Lint & Type Check, CodeQL / Analyze python+actions) on 1687ec0 (merge of origin/main into fbd5f4b; PR's src/tests/scripts diff vs main is byte-identical to the reviewed one, main's new changes touch only linear/whatsapp/ai; only CHANGELOG conflicted, both sides kept; fidelity target unchanged 151 -> 151). Local Codex review (gpt-6-astra, xhigh, --base origin/main) on fbd5f4b: "No actionable regressions found relative to the supplied merge base. All 508 Telegram tests passed, and changed-file lint and formatting checks passed." 6 astra rounds; CodeRabbit rate-limited (no review), no other bot findings. Merging with --admin (Protect Main requires a code-owner approval).