fix(telegram): close reconnect races that leave adapter half-destroyed

[JoaoMarcos44]

Fixes #55992

Summary

After a network blip, the Telegram adapter self-restarts, logs ✓ telegram connected, and then the gateway silently stops processing any further messages — while gateway_state.json keeps reporting connected/running and the process itself stays alive (or, on Windows, dies with a non-standard exit code). This PR fixes the root cause: a pair of concurrency bugs that let more than one recovery attempt run against the same Telegram adapter instance at once.

Root cause

Three issues combine to produce the symptom:

1. Stale reentrancy guard (plugins/platforms/telegram/adapter.py, _handle_polling_network_error)

When start_polling() fails during a reconnect attempt, the handler self-schedules a follow-up retry via asyncio.ensure_future(...), but that task was only tracked in self._background_tasks — it was never assigned to self._polling_error_task. That field is the only reentrancy guard checked by three independent trigger points: the PTB error_callback, _polling_heartbeat_loop (runs every 90s), and _probe_pending_updates. With the guard going stale while a retry was still in flight, any of those three could start a second, fully independent recovery attempt for the same underlying outage, sharing the same self._app with no synchronization.

2. TOCTOU window in the fatal-error handler (gateway/run.py, _handle_adapter_fatal_error)

existing = self.adapters.get(adapter.platform)
if existing is adapter:
    try:
        await adapter.disconnect()
    finally:
        self.adapters.pop(adapter.platform, None)

The adapter was only removed from self.adapters in the finally block, after awaiting disconnect(). If issue #1 above caused a second concurrent fatal-error notification for the same adapter object, that second call would still see existing is adapter == True (because the first call hadn't reached its finally yet) and would call disconnect() a second time on the same object.

That double disconnect() is the concrete origin of the 'NoneType' object has no attribute 'updater' exception reported in the issue: the first disconnect() call sets self._app = None at the end of its teardown, and the second concurrent call — already past its own if self._app and self._app.updater... check — re-reads self._app a few lines later and finds None.

Net effect: two live PTB Application/Updater objects (or one live + one half-torn-down) can end up racing for the same bot token's getUpdates long-poll. The Updater that loses that race sits in a healthy-looking running=True state, receiving empty long-poll responses forever, with no exception ever raised — which is why the log goes completely silent and gateway_state.json never updates again (it's written once, on the connect() that "won"). This also explains why the failure is intermittent rather than deterministic: it depends on the exact interleaving of the heartbeat loop, the pending-updates probe, and the network-error callback around the same outage.

3. Stale notifications outliving their adapter (gateway/run.py, _handle_adapter_fatal_error)

Even after closing #1 and #2, a delayed fatal-error notification can still arrive from an adapter instance that has already been replaced — e.g. its own background retry chain (started before the reconnect watcher successfully installed a fresh adapter) finally exhausts its retries and reports failure after the fact. The handler processed that notification unconditionally: it overwrote the platform's runtime status back to retrying/fatal even though a different, healthy adapter was already running, and could re-queue an already-connected platform into _failed_platforms for reconnection it didn't need.

Fix

Three small, localized changes — no architectural changes, no behavior change to the retry/backoff strategy itself:

1. plugins/platforms/telegram/adapter.py — the chained retry task created in _handle_polling_network_error's except block is now assigned to self._polling_error_task, so the shared reentrancy guard correctly reflects that a recovery is still in flight. Also captures self._app in a local variable at the top of the stop/start_polling sequence and reuses that reference across both await points, instead of re-reading self._app (which a concurrent disconnect() could have already cleared).
2. gateway/run.py — _handle_adapter_fatal_error now pops the adapter out of self.adapters (and updates delivery_router.adapters) before awaiting disconnect(), not after. A second concurrent notification for the same adapter now correctly sees it's already been claimed and skips the redundant disconnect() call.
3. gateway/run.py — _handle_adapter_fatal_error now snapshots the current owner of the platform slot as its very first step, before any log line or status write. If that owner is neither None nor the adapter reporting the error, the notification is stale (a different adapter already owns the slot) and the handler returns immediately, before touching runtime status or the reconnect queue.

Steps to reproduce (from the issue)

1. Gateway is happily polling Telegram.
2. Network hiccup — httpx.ConnectError fires a few times in a row.
3. After 10 consecutive failures, the adapter logs Fatal telegram adapter error → Restarting gateway.
4. Reconnect happens, log says ✓ telegram connected.
5. Gateway never processes another message. Log goes completely dead, but the process is still alive, still responds to SIGTERM, and gateway_state.json still says running/connected. On Docker this has been observed to last 6h9m+ of total silence; on Windows the process exits instead (non-standard exit code) with the state file stuck at starting.

This is a timing-dependent race, so it isn't reliably reproducible with a fixed manual recipe — the two regression tests below reproduce the exact interleavings that trigger it.

Testing

Added three regression tests that fail against the pre-fix code and pass against this change:

• tests/gateway/test_telegram_network_reconnect.py::test_reconnect_chained_retry_updates_polling_error_task — asserts that after a failed start_polling() retry, adapter._polling_error_task points at the newly scheduled task (not stale/done), closing the reentrancy-guard gap described in root cause Terminal tool #1.
• tests/gateway/test_runner_fatal_adapter.py::test_concurrent_fatal_notifications_disconnect_same_adapter_once — drives two concurrent _handle_adapter_fatal_error(adapter) calls for the same adapter object (via asyncio.gather, with a mocked disconnect() that yields control mid-teardown) and asserts disconnect() is called exactly once. Without the fix, this test observes 2 calls.
• tests/gateway/test_runner_fatal_adapter.py::test_stale_fatal_notification_from_superseded_adapter_is_ignored — installs a healthy adapter, then feeds a fatal-error notification from a different, superseded adapter instance for the same platform, and asserts the healthy adapter is left untouched (disconnect() never called on it), the platform is not re-queued into _failed_platforms, and the gateway doesn't shut down. Without the fix, the stale notification is processed as if it came from the live adapter.

Ran:

pytest tests/gateway/test_telegram_network_reconnect.py tests/gateway/test_runner_fatal_adapter.py \
       tests/gateway/test_telegram_conflict.py tests/gateway/test_telegram_pending_update_probe.py \
       tests/gateway/test_platform_reconnect.py -v

All 81 tests pass, including the 3 new regression tests. Also ran the full tests/gateway/ suite; the only failures present are pre-existing and unrelated to this change (confirmed via git stash — they reproduce identically on main, e.g. Windows-vs-Unix path assumptions in test_status.py).

Out of scope

• gateway/status.py's one-shot (event-driven, not periodic) connected state write is a separate, larger design question (should there be a periodic "still healthy" re-validation?) and is intentionally not addressed here to keep this fix minimal and low-risk.
• _handle_polling_conflict (the 409-conflict retry ladder) has a structurally similar pattern to the network-error ladder and may warrant the same local-self._app-capture treatment, but wasn't implicated in the reported symptom and is left for a follow-up if confirmed necessary.

[alt-glitch]

This was generated by AI during triage.

Competing with #56028 for the same issue (#55992). #56028 is the narrow defense-in-depth guard (None-check on self._app.updater before start_polling()); this PR fixes the underlying concurrency races that produce the NoneType in the first place (stale _polling_error_task reentrancy guard + the _handle_adapter_fatal_error TOCTOU in gateway/run.py). Cross-linking so a maintainer can pick the canonical fix.

[teknium1]

Merged via PR #56200 — your two commits were rebased onto current main with your authorship preserved in git log. Thanks for the root-cause fix (claim-slot-before-disconnect, stable-local/fail-fast, reentrancy-guard rearm). We added one follow-up commit widening the same guard to the 409-conflict retry path. #56200

[kshitijk4poor]

Superseded by #56224, which carries both your commits verbatim (authorship preserved) rebased onto current main (this PR was 127 commits behind), with our full review workflow run on it (concurrency + correctness + manual trace — all clean, 27 tests + adjacent gateway suites pass). Full credit to you for the fix. Closing in favor of #56224 — feel free to push back.