fix(gateway): hand Telegram held inbound to the rebuilt adapter

Telegram holds inbound events that PTB has already acked (offset advanced),
so dropping them means losing the message for good. The hold queue lives on
the adapter instance and drains only on that instance's _mark_connected. When
a retryable fatal (network error, polling/ingress stall) makes the runner
build a NEW adapter, that drain never runs: the reconnect path carried the
inbound dedup caches across but not the queue. Held events, plus anything
teardown salvaged onto the retired instance later, were silently stranded.

The reconnect queue entry (primary) and the secondary reconnect task now
keep the replaced adapter as "predecessor". The existing carry_inbound_dedup
seam reads its dedup caches from it before connect. A new
hand_over_held_inbound seam moves its held events at publish time
(_install_reconnected_adapter / secondary profile_map install). A candidate
that fails to connect never owns the queue, so no hand-back is needed, and
delivery waits until the replacement is registered. TelegramAdapter.
adopt_held_inbound rebinds only source._transport_adapter_ref (profile and
role authorization stay as resolved) and leaves a successor weakref on the
predecessor. Late _hold_inbound_event calls there (teardown salvage after the
replacement drained) forward to the replacement.

Reported-by: j2h4u (Maxim Brashenko)
Fixes #132829
This commit is contained in:
kshitijk4poor
2026-10-05 13:43:49 +05:30
committed by kshitij
parent f432912636
commit 7ef5aca075
5 changed files with 105 additions and 12 deletions

View File

@@ -72,18 +72,28 @@ def inbound_dedup_caches(adapter: Any) -> dict[str, MessageDeduplicator]:
return {name: v for name, v in vars(adapter).items() if isinstance(v, MessageDeduplicator)}
def carry_inbound_dedup(caches: Optional[dict], adapter: Any) -> None:
"""Seed a rebuilt adapter's dedup caches from the instance it replaces.
def carry_inbound_dedup(predecessor: Any, adapter: Any) -> None:
"""Seed a rebuilt adapter's dedup caches from the instance it replaces (call before connect).
The runner's reconnect path builds a NEW adapter; without this a platform replaying a recent
inbound ID after the reconnect (websocket resume, webhook retry, unacked poll batch) is
admitted and answered a second time."""
for name, previous in (caches or {}).items():
for name, previous in (inbound_dedup_caches(predecessor) if predecessor is not None else {}).items():
current = getattr(adapter, name, None)
if isinstance(current, MessageDeduplicator) and current is not previous:
current.absorb(previous)
def hand_over_held_inbound(predecessor: Any, adapter: Any) -> None:
"""Move inbound the retired instance is holding to its now-published replacement (#132829).
Runs at publish time, not before connect: a candidate that fails to connect never owns the
queue, and the predecessor keeps holding until a replacement is registered."""
adopt = getattr(adapter, "adopt_held_inbound", None)
if predecessor is not None and callable(adopt):
adopt(predecessor)
# Worker-thread handoff used by the off-loop persist paths. A module attribute
# so tests can replace THIS seam instead of patching ``asyncio.to_thread``
# globally.

View File

@@ -26,7 +26,7 @@ from gateway.config import (
platform_binds_port as _platform_binds_port,
)
from gateway.platforms.base import BasePlatformAdapter
from gateway.platforms.helpers import carry_inbound_dedup, inbound_dedup_caches
from gateway.platforms.helpers import carry_inbound_dedup, hand_over_held_inbound
from gateway.restart import is_global_startup_conflict
from gateway.run_shutdown import _log_suppressed
from gateway.session import SessionSource
@@ -244,7 +244,8 @@ class GatewayAdapterLifecycleMixin:
**({"queued_at": now} if queued else {}),
"credential_claim": self._adapter_credential_claim(platform, adapter),
"listener_claim": self._adapter_listener_claim(platform, adapter),
"inbound_dedup": inbound_dedup_caches(adapter) if adapter is not None else None,
# The replaced instance: its dedup caches seed each candidate, its held inbound moves on publish.
"predecessor": adapter,
}
def _queue_retryable_fatal_platform(self, adapter: BasePlatformAdapter) -> bool:
@@ -827,7 +828,7 @@ class GatewayAdapterLifecycleMixin:
)
logger.info("Reconnect %s: no adapter yet, next retry in %ds", platform.value, backoff)
return
carry_inbound_dedup(info.get("inbound_dedup"), adapter)
carry_inbound_dedup(info.get("predecessor"), adapter)
self._wire_adapter_handlers(adapter)
# is_reconnect keeps the server-side update queue so offline-period messages are delivered.
success = await self._connect_adapter_with_timeout(adapter, platform, is_reconnect=True)
@@ -891,7 +892,7 @@ class GatewayAdapterLifecycleMixin:
"""Publish a freshly reconnected primary adapter and replay what it missed while down."""
self._publish_primary_adapter(platform, adapter)
self.delivery_router.adapters = self.adapters
del self._failed_platforms[platform]
hand_over_held_inbound(self._failed_platforms.pop(platform).get("predecessor"), adapter)
# connect() returning True does not mean the receive path is confirmed -- Telegram's degraded
# reconnect returns True so the gateway stays up while its own ladder retries. Stamping "connected"
# here would undo the adapter's accurate status.
@@ -1398,7 +1399,7 @@ class GatewayAdapterLifecycleMixin:
and _platform_binds_port(platform.value, getattr(getattr(adapter, "config", None), "extra", None)):
adapter._shared_listener_profile = profile_name
async def _secondary_reconnect_attempt(self, profile_name: str, platform: Platform, inbound_dedup=None):
async def _secondary_reconnect_attempt(self, profile_name: str, platform: Platform, predecessor=None):
"""One scoped attempt to rebuild+connect a secondary adapter → ``(adapter, success)``;
``(None, None)`` = give up for good (disabled, credential removed, adapter unavailable). Caller
tears down a RETURNED adapter; one whose configure/connect raised is torn down here."""
@@ -1430,7 +1431,7 @@ class GatewayAdapterLifecycleMixin:
platform.value, profile_name,
)
return None, None
carry_inbound_dedup(inbound_dedup, adapter)
carry_inbound_dedup(predecessor, adapter)
try:
self._configure_profile_adapter(adapter, profile_name, platform)
success = await self._connect_adapter_with_timeout(adapter, platform, is_reconnect=True)
@@ -1441,7 +1442,7 @@ class GatewayAdapterLifecycleMixin:
return adapter, success
async def _run_secondary_profile_reconnect(
self, profile_name: str, platform: Platform, inbound_dedup=None
self, profile_name: str, platform: Platform, predecessor=None
) -> None:
"""Reconnect a retryable secondary adapter under its own profile scope."""
from gateway.run import _profile_runtime_scope, _reconnect_backoff
@@ -1454,7 +1455,7 @@ class GatewayAdapterLifecycleMixin:
adapter = None
try:
adapter, success = await self._secondary_reconnect_attempt(
profile_name, platform, inbound_dedup
profile_name, platform, predecessor
)
if adapter is None:
return
@@ -1462,6 +1463,7 @@ class GatewayAdapterLifecycleMixin:
profile_map = self._profile_adapters.setdefault(profile_name, {})
if platform not in profile_map:
profile_map[platform] = adapter
hand_over_held_inbound(predecessor, adapter)
self._sync_voice_mode_state_to_adapter(adapter)
logger.info("✓ %s reconnected (profile: %s)", platform.value, profile_name)
await self._redeliver_failed_obligations_for_platform(
@@ -1576,7 +1578,7 @@ class GatewayAdapterLifecycleMixin:
if platform in profile_pending:
return
profile_pending[platform] = self._retain_background_task(asyncio.create_task(
self._run_secondary_profile_reconnect(profile_name, platform, inbound_dedup_caches(adapter)),
self._run_secondary_profile_reconnect(profile_name, platform, adapter),
name=f"secondary-reconnect:{profile_name}:{platform.value}",
))

View File

@@ -10,6 +10,7 @@ import os
import html as _html
import re
import time
import weakref
from contextvars import ContextVar
from datetime import datetime, timezone
from typing import Any, Awaitable, Callable, Dict, Iterator, List, Optional, Set
@@ -818,6 +819,10 @@ class TelegramAdapter(BasePlatformAdapter):
logger.warning(
"[Telegram] Discarding inbound under non-retryable fatal (%s, %d chars)", where, len(getattr(event, "text", None) or ""))
return
successor = getattr(self, "_held_inbound_successor", None)
if successor is not None and successor() is not None: # retired by a runner rebuild (#132829)
successor()._adopt_held_event(event, where=f"{where}-forwarded", schedule=schedule)
return
held = getattr(self, "_held_inbound_events", None)
if held is None:
self._held_inbound_events = held = []
@@ -837,6 +842,24 @@ class TelegramAdapter(BasePlatformAdapter):
if schedule and not self._should_drop_delayed_delivery():
self._schedule_held_inbound_redispatch()
def adopt_held_inbound(self, predecessor: "TelegramAdapter") -> None:
"""Take over the hold queue of the instance the runner just replaced with us (#132829): it only
drains on its own ``_mark_connected``, which never comes; later holds there forward here."""
if predecessor is self or not isinstance(predecessor, TelegramAdapter):
return
predecessor._held_inbound_successor = weakref.ref(self)
held = getattr(predecessor, "_held_inbound_events", None) or []
events, held[:] = list(held), []
for event in events:
self._adopt_held_event(event, where="adopted", schedule=False)
self._schedule_held_inbound_redispatch()
def _adopt_held_event(self, event: "MessageEvent", *, where: str, schedule: bool) -> None:
"""Hold a predecessor's event here; only the transport ref moves (profile/authz stay as resolved)."""
if getattr(event, "source", None) is not None:
event.source._transport_adapter_ref = weakref.ref(self)
self._hold_inbound_event(event, where=where, schedule=schedule)
def _rehold_from(self, events: list, idx: int, where: str) -> None:
"""Re-hold ``events[idx:]`` without rescheduling (drain interrupted / failed / cancelled)."""
for rest in events[idx:]:

View File

@@ -412,6 +412,49 @@ class TestReconnectKeepsInboundDedup:
assert new._dedup.is_duplicate("m1") is True
assert new._dedup.is_duplicate("m2") is False
@pytest.mark.asyncio
async def test_held_telegram_inbound_reaches_published_replacement_once(self):
"""PTB already acked a held update, so the queue on the retired Telegram adapter must be
delivered exactly once by the replacement the watcher publishes, not by a failed candidate (#132829)."""
from gateway.platforms.event import MessageEvent
from gateway.session import SessionSource
from plugins.platforms.telegram.adapter import TelegramAdapter
class _Telegram(TelegramAdapter):
def __init__(self, succeed):
super().__init__(PlatformConfig(enabled=True, token="123:abc"))
self.succeed, self.handle_message = succeed, AsyncMock()
async def connect(self, *, is_reconnect=False):
if self.succeed:
self._mark_connected()
return self.succeed
async def disconnect(self):
return None
runner = _make_runner()
runner.stop = AsyncMock()
runner._sync_voice_mode_state_to_adapter = MagicMock()
old, failed, new = _Telegram(True), _Telegram(False), _Telegram(True)
runner.adapters[Platform.TELEGRAM] = old
old._set_fatal_error("telegram_network_error", "stall", retryable=True)
event = MessageEvent(text="held", source=SessionSource(platform=Platform.TELEGRAM, chat_id="1", chat_type="dm"))
old._hold_inbound_event(event, where="text-enqueue")
await runner._handle_adapter_fatal_error(old)
for candidate in (failed, new):
runner._failed_platforms[Platform.TELEGRAM]["next_retry"] = 0
with patch.object(runner, "_create_adapter", return_value=candidate):
await runner._reconnect_failed_platform(Platform.TELEGRAM, time.monotonic() + 1)
await asyncio.sleep(0)
await new._held_inbound_redispatch_task
assert runner.adapters[Platform.TELEGRAM] is new
failed.handle_message.assert_not_called()
new.handle_message.assert_awaited_once_with(event)
assert event.source._transport_adapter_ref() is new
assert old._held_inbound_events == [] and new._held_inbound_events == []
# --- Pause / resume circuit breaker ---

View File

@@ -195,6 +195,21 @@ class TestHoldInboundAcrossReconnect:
assert adapter.handle_message.call_args[0][0].text == "should survive disconnect"
assert adapter._held_inbound_events == []
@pytest.mark.asyncio
async def test_late_teardown_salvage_on_retired_adapter_reaches_replacement(self):
"""Teardown of a rebuilt-away adapter can salvage a batch after the replacement drained (#132829)."""
old, new = _make_adapter(), _make_adapter()
old._mark_disconnected()
new.adopt_held_inbound(old)
old._pending_text_batches["k"] = _make_event("late-salvage")
await old._cancel_pending_delivery_tasks()
await new._held_inbound_redispatch_task
old.handle_message.assert_not_called()
assert [c.args[0].text for c in new.handle_message.call_args_list] == ["late-salvage"]
assert old._held_inbound_events == []
@pytest.mark.asyncio
async def test_flush_during_disconnect_holds_popped_event(self):
"""After pop, drop-guard must hold — not destroy — the event.