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

[hashbender]

Summary

Salvage of NousResearch#56036 by @JoaoMarcos44 (rebased onto current main). Closes the concurrency races that leave the Telegram adapter "half-destroyed" — the gateway logs ✓ telegram connected but silently stops processing messages after a network blip + self-restart, and can crash with 'NoneType' object has no attribute 'updater' (issue NousResearch#55992).

Original PR was 127 commits behind main; both of @JoaoMarcos44's commits are cherry-picked here verbatim (authorship preserved).

The bug

After a network outage, multiple recovery paths (the PTB error callback, the heartbeat loop, the pending-updates probe, and the gateway's fatal-error handler) can race on the same outage:

1. gateway/run.py::_handle_adapter_fatal_error TOCTOU — the adapter was removed from self.adapters in a finally block after await disconnect(). A second concurrent fatal-error notification for the same adapter still saw itself as the installed adapter during that await and called disconnect() on the same object twice — the concrete origin of the NoneType ... updater crash when teardown re-reads self._app.
2. telegram/adapter.py::_handle_polling_network_error — read self._app repeatedly across await points; a concurrent disconnect() setting self._app = None mid-sequence swapped in None silently. The chained retry also didn't update self._polling_error_task, so the reentrancy guard went stale and each recovery path could start its own concurrent recovery for the same outage.

The fix

• run.py: snapshot the platform-slot owner first and ignore a stale fatal notification from a superseded adapter instance (a background retry chain that lost to an already-succeeded reconnect); and claim the adapter (pop from self.adapters before awaiting disconnect()) so a concurrent notification can't double-disconnect.
• adapter.py: capture a stable app = self._app local before the awaits and operate on it (fail fast with a RuntimeError if it was torn down, instead of an AttributeError on None.updater); and set self._polling_error_task = task for the chained retry so the reentrancy guard stays valid while recovery is in flight.

Review (this salvage)

Ran our review workflow — a concurrency/race pass, a correctness/scope pass, and my own trace. Clean, no findings.

• Race analysis: no await sits between the pop and await disconnect() (no dropped-delivery window; strictly better than the old ordering, which kept a half-disconnected adapter routable for the whole disconnect). The stale-notification early-return uses an identity check (existing is not adapter) so it only suppresses superseded instances, never a legitimate same-object error. After app = self._app, there are no residual self._app reads before start_polling() in the method. The _polling_error_task handoff forms a proper chain — assigned synchronously with task creation (no yield between), cleared on teardown.
• RuntimeError is caught: the new raise RuntimeError(...) before start_polling() is caught by the method's own except Exception handler, which re-enters the retry ladder — it never propagates uncaught to the direct-await or background-task callers.
• Scope: clean — 2 code files + 2 test files, all on the reconnect-race fix.

Tests

pytest tests/gateway/test_runner_fatal_adapter.py tests/gateway/test_telegram_network_reconnect.py -q
27 passed

New tests cover: two concurrent fatal notifications → exactly one disconnect(); a stale notification from a superseded adapter is ignored (no disconnect of the new adapter, no re-queue, no shutdown); and the chained retry updating _polling_error_task. Adjacent gateway suites (delivery, runner-startup, notice-delivery) also pass — no regressions.

Supersedes NousResearch#56036 and NousResearch#56028 (@AlexFucuson9's narrower None-guard for the same NousResearch#55992 symptom — this PR fixes the underlying races that produce the NoneType in the first place). Full credit to @JoaoMarcos44.

---

Mirror-of: NousResearch#56224
NousResearch#56224

[tenki-reviewer]

Review Complete

Files Reviewed: 2
Findings: 1

By Severity:

• 🟡 Medium: 1

---

This PR introduces two regressions: a blocking sync HTTP call in the async gateway event loop (high severity) and a removed concurrency guard in Telegram polling conflict recovery (medium severity). Both revert previously-fixed issues.

Files Reviewed (2 files)

gateway/run.py
plugins/platforms/telegram/adapter.py

[tenki-reviewer]

Risk: 🟠 High (75/100) — 1 medium finding · 50 LOC across 2 files

---

Summary

This 2-file PR modifies gateway/run.py and plugins/platforms/telegram/adapter.py, introducing two regressions that revert previously-fixed issues.

finding-001 (high): Blocking sync HTTP call in async gateway event loop

Two call sites in gateway/run.py were changed from await get_model_context_length_async(...) to the synchronous get_model_context_length(...). Both run in the gateway's asyncio event loop — one on every inbound message with @ references, the other on every message when history reaches 4+ messages. The sync function performs blocking HTTP requests via the requests library (probing /v1/models, Ollama /api/show, OpenRouter APIs, etc.) plus blocking YAML file I/O. When the 300s in-memory cache misses or expires, the event loop freezes for all connected platforms simultaneously — Discord/Slack WebSocket timeouts, Telegram polling stalls, message processing halts. The async wrapper (get_model_context_length_async) uses asyncio.to_thread() to safely offload this work and still exists in the codebase but has no remaining callers.

finding-002 (medium): Removed concurrency guard in Telegram polling conflict recovery

In _handle_polling_conflict, the local-reference snapshot app = self._app and its null-check guard were removed. This pattern was explicitly documented as protecting against a concurrent disconnect() reassigning self._app to None during async suspension points (issue NousResearch#55992). The sibling method _handle_polling_network_error still uses the identical pattern. Without it, a shutdown-triggered disconnect() concurrent with conflict recovery causes an AttributeError. Impact is bounded — the except handler catches it and reschedules a retry — but diagnostic quality degrades and a known concurrency defect is reintroduced.

Risk Assessment

The event-loop blocking regression (finding-001) is the critical concern: it can freeze the entire gateway across all platforms when model metadata caches expire. The Telegram concurrency regression (finding-002) compounds reliability risk in the polling error-recovery path.

[tenki-reviewer]

🟡 Removed local-reference snapshot reintroduces race condition in Telegram polling conflict recovery (bug)

In _handle_polling_conflict at plugins/platforms/telegram/adapter.py, the patch removed the local variable snapshot app = self._app that was captured before two await points (lines 2165-2166: asyncio.sleep(RETRY_DELAY) and await self._drain_polling_connections()). The removed code comments explicitly documented that self._app can be reassigned to None by a concurrent disconnect() during those awaits, citing issue NousResearch#55992.

After the patch, line 2169 reads self._app directly: await self._app.updater.start_polling(...). If disconnect() (line 3042: self._app = None) runs concurrently during the await suspension — e.g., from a gateway shutdown calling stop() → _bounded_adapter_teardown → disconnect() — accessing self._app.updater raises AttributeError: 'NoneType' object has no attribute 'updater'.

The sibling method _handle_polling_network_error (line 1780) still preserves the exact same capture pattern with an explicit comment about the concurrent disconnect() race, confirming the pattern was deliberately applied to protect async re-entrancy in PTB error callbacks.

Impact is bounded because the except Exception handler at line 2180 catches both the old RuntimeError and the new AttributeError, rescheduling a retry. The regression is primarily in diagnostic quality (unclear error message vs. explicit 'Telegram application was torn down') and the reintroduction of a known, previously-fixed race pattern.

💡 Suggestion: Restore the local-reference snapshot pattern that was removed: before the try block at line 2168, capture app = self._app. Then inside the try, guard with if not app: raise RuntimeError('Telegram application was torn down during conflict reconnect') and use app.updater.start_polling(...) instead of self._app.updater.start_polling(...). This matches the proven pattern still present in the sibling method _handle_polling_network_error at lines 1780-1793.

📋 Prompt for AI Agents

In plugins/platforms/telegram/adapter.py, restore the concurrency guard in _handle_polling_conflict:

1. Before line 2168 (try:), add: app = self._app
2. At line 2168-2169, replace try:\n await self._app.updater.start_polling( with:

try:
    if not app:
        raise RuntimeError("Telegram application was torn down during conflict reconnect")
    await app.updater.start_polling(

This restores the local-reference snapshot pattern that protects against self._app being set to None by a concurrent disconnect() during the await points above. The sibling method _handle_polling_network_error at line 1780 still uses this pattern with the same documented rationale.