Skip to content

fix(telegram): wall-deadline init timeout + shut down abandoned init app - #58293

Merged
kshitijk4poor merged 2 commits into
NousResearch:mainfrom
kshitijk4poor:salvage/telegram-init-deadline
Jul 4, 2026
Merged

fix(telegram): wall-deadline init timeout + shut down abandoned init app#58293
kshitijk4poor merged 2 commits into
NousResearch:mainfrom
kshitijk4poor:salvage/telegram-init-deadline

Conversation

@kshitijk4poor

@kshitijk4poor kshitijk4poor commented Jul 4, 2026

Copy link
Copy Markdown
Collaborator

Summary

Salvage of #58250 (@msh01) onto current main, plus a follow-up that closes a resource leak the abandonment path introduced and covers the new helper directly.

Fixes #58236: under s6 supervision, asyncio.wait_for(self._app.initialize(), timeout=30) never fires because httpcore's initialize() sits inside anyio shielded CancelScopes, so the timeout's cancellation never propagates — the bot hangs on "attempt 1/8" forever. (The reporter confirmed a naive Timer + task.cancel() still hangs, because it awaits cancellation.)

Changes

  • @msh01's commit (preserved): replace wait_for with _await_with_thread_deadline() — a daemon threading.Timer wakes the loop and, on timeout, abandons the wedged task (cancels without awaiting cancellation) and raises TimeoutError so the 8-attempt retry ladder advances instead of hanging.
  • Follow-up (this salvage):
    • Resource leak: the abandoned initialize() leaves the half-built PTB app's httpx client / connection pool open (never awaited to completion), leaking a pool per retry attempt (up to 8×). Add an optional on_abandon cleanup to the helper, run detached + exception-swallowed so it can never re-block or re-hang the ladder (mirrors _close_client_on_timeout in agent/auxiliary_client.py).
    • Leak-fix must actually release the pool: Application.shutdown()/Bot.shutdown() are gated on _initialized/_requests_initialized, which a wedged initialize() may never set — so calling only app.shutdown() no-ops and leaks anyway. _shutdown_abandoned_app() tries the clean app.shutdown() first, then falls back to closing each bot._request transport directly (HTTPXRequest.shutdown() gates only on client.is_closed, and the client is built eagerly in the constructor), so the pool is released regardless of PTB init state.
    • Test coverage: the salvaged test monkeypatched out the real helper. Add direct tests for happy-path return, prompt-timeout-with-cleanup, cleanup-error-swallowed, and the uninitialized-app transport-close path (the leak-fix effectiveness case) + a robustness test for None/missing-_request. Wedged coroutines swallow cancellation for a bounded window (proving the helper returns before cancellation completes) without leaving an immortal task that would wedge pytest teardown. Widen the salvaged stub to accept on_abandon.
    • Attribution: add yingwaizhiying@gmail.com → msh01 to AUTHOR_MAP (bare gmail doesn't auto-resolve the attribution gate).

Known follow-up (not in this PR)

The retry ladder reuses the same self._app across all 8 attempts. If an abandoned initialize() completes in the background, re-initialize() on that same app is a coherence risk (distinct from the pool leak, which this PR closes). The full fix (fresh _app per attempt) is a larger restructure of the ~130-line builder+handler setup — left for a separate change.

Validation

  • tests/gateway/test_telegram_init_deadline.py — 6 passed (original ladder-retry test + 5 helper/leak-fix tests).
  • Sibling telegram suite (closewait_limits, network, network_reconnect, send_path_health) — 86 passed total.
  • Phase 2b dataflow trace: happy-path return; prompt timeout (<0.8s vs a ~1s wedged coroutine); abandoned task cancelled with no "exception never retrieved" (verified in -X dev); cleanup fires on timeout; cleanup-raise (sync + async) swallowed; None/non-awaitable returns handled; task-wins race returns the real value and never calls cleanup.
  • ruff clean; ty lint-diff 🆕 New issues: none; check-attribution passes.

Closes #58250. Credit to @msh01 — their commit is cherry-picked with authorship preserved.

@alt-glitch alt-glitch added type/bug Something isn't working comp/gateway Gateway runner, session dispatch, delivery platform/telegram Telegram bot adapter sweeper:risk-message-delivery Sweeper risk: may drop, duplicate, misroute, or suppress messages P1 High — major feature broken, no workaround labels Jul 4, 2026
…deadline helper

Follow-up to @msh01's wall-deadline init-timeout fix.

- Resource leak: on timeout the initialize() task is abandoned without
  awaiting its (shielded, possibly-never-completing) cancellation, so the
  half-built PTB app's httpx client / connection pool was never closed —
  up to 8x across the retry ladder. Add an optional on_abandon cleanup to
  _await_with_thread_deadline that best-effort app.shutdown()s the abandoned
  app, run detached + exception-swallowed so it can never re-block or re-hang
  the ladder (mirrors _close_client_on_timeout in agent/auxiliary_client.py).
- Cover the helper itself: the salvaged test monkeypatched out the real
  _await_with_thread_deadline, so its abandonment/cleanup path was untested.
  Add direct tests for happy-path return, prompt-timeout-with-cleanup, and
  cleanup-error-swallowed; the wedged coroutines swallow cancellation for a
  bounded window (proving the helper returns before cancellation completes,
  the NousResearch#58236 shielded-scope behavior) without leaving an immortal task that
  would wedge pytest teardown. Widen the salvaged stub to accept on_abandon.
- Attribution: add yingwaizhiying@gmail.com -> msh01 to AUTHOR_MAP (bare
  gmail does not auto-resolve the check-attribution gate).

Known follow-up (not addressed here): the retry ladder reuses the same
self._app across all 8 attempts; a fresh app per attempt would fully close
the coherence risk if an abandoned initialize() completes in the background.
That is a larger restructure of the ~130-line builder+handler setup, left
for a separate change.
@kshitijk4poor
kshitijk4poor force-pushed the salvage/telegram-init-deadline branch from 067e063 to a37fd66 Compare July 4, 2026 15:16
@kshitijk4poor
kshitijk4poor enabled auto-merge July 4, 2026 15:16
@kshitijk4poor
kshitijk4poor merged commit 5daa5a0 into NousResearch:main Jul 4, 2026
29 checks passed

@tonydwb tonydwb left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Code Review Summary

Verdict: Comment

Uses wall-clock deadline instead of monotonic timer for the Telegram init timeout. The old approach could misbehave if the system clock changed mid-session; wall-clock is more reliable for absolute timeouts.

Observations

  • The salvage PR references jonathan.kovacs999@gmail.com in the author map — this appears to be a test data entry in a fixture, not an actual credential exposure. Confirmed safe.
  • The chainable.token.return_value = chainable pattern is a mock setup for testing — appropriate.

Looks Good

  • No hardcoded secrets
  • No debug artifacts

Reviewed by Hermes Agent

habarmc1223-sudo pushed a commit to habarmc1223-sudo/hermes-agent-fluxmem that referenced this pull request Jul 8, 2026
…gram-init-deadline

fix(telegram): wall-deadline init timeout + shut down abandoned init app
santhreal pushed a commit to santhreal/hermes-agent that referenced this pull request Jul 13, 2026
…gram-init-deadline

fix(telegram): wall-deadline init timeout + shut down abandoned init app
Gravezzz pushed a commit to Gravezzz/hermes-agent that referenced this pull request Jul 21, 2026
…gram-init-deadline

fix(telegram): wall-deadline init timeout + shut down abandoned init app
@kshitijk4poor
kshitijk4poor deleted the salvage/telegram-init-deadline branch August 5, 2026 07:10
leewenjie pushed a commit to leewenjie/hermes-agent that referenced this pull request Aug 7, 2026
…gram-init-deadline

fix(telegram): wall-deadline init timeout + shut down abandoned init app
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

comp/gateway Gateway runner, session dispatch, delivery P1 High — major feature broken, no workaround platform/telegram Telegram bot adapter sweeper:risk-message-delivery Sweeper risk: may drop, duplicate, misroute, or suppress messages type/bug Something isn't working

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Gateway hangs indefinitely on Telegram connect (attempt 1/8) — asyncio.wait_for never fires under s6 supervision

4 participants