fix(telegram): prevent connect hang with retry watchdog and fresh app per attempt (#67498) - #70546
Closed
webtecnica wants to merge 2 commits into
Closed
fix(telegram): prevent connect hang with retry watchdog and fresh app per attempt (#67498)#70546webtecnica wants to merge 2 commits into
webtecnica wants to merge 2 commits into
Conversation
added 2 commits
July 24, 2026 01:16
…ker deadlock on server backgrounding Issue NousResearch#68915: when the agent runs a compound command with trailing & (e.g. `cd /app && node server.js &`), bash parses it as `(A && B) &` — a subshell that holds the stdout pipe open forever when B is a long-running server. The existing _rewrite_compound_background in terminal_tool.py correctly rewrites this to `A && { B & }` to avoid the subshell fork, but it was only applied in the foreground execute() path (tools/environments/base.py). The background spawn_local() path bypasses base.py entirely and passed the raw command directly to Popen/PTY, leaving the deadlock unmitigated. Fix: apply _rewrite_compound_background in spawn_local() before the command is passed to Popen or PTY spawn. Uses a lazy import to avoid circular dependency (terminal_tool imports process_registry). - PTY spawn path: now uses safe_command (rewritten) - Popen spawn path: now uses safe_command (rewritten) - Session.command still stores the original (unrewritten) command for display - Simple `cmd &` is left unchanged (no subshell bug) Tests: 4 regression tests verifying (1) compound is rewritten, (2) simple bg is preserved, (3) multi-line compounds are rewritten, (4) session.command stores original.
… per attempt (NousResearch#67498) The Telegram adapter's connect retry loop could silently stall after 'Connecting to Telegram (attempt 1/8)...' with the event loop permanently parked in select() — all threads idle, no attempt 2/8 ever scheduled. Root cause analysis: - The retry loop reused the same Application object across all 8 attempts. After a failed initialize() the app could be in a partially- initialized state (closed httpx transports from , or flag set before the hang) causing subsequent calls to silently skip real initialization. - CancelledError (a BaseException, not an Exception) propagated silently through all except handlers with no logging — the task driving the retry loop could exit without any trace. - No total watchdog bound existed for the entire retry loop; only per-attempt timeouts via _await_with_thread_deadline. If the loop itself stalled between attempts (between-attempt sleep, cleanup, or scheduling), there was no timeout to catch it. Fixes: 1. **Total watchdog deadline**: Compute a total deadline for the entire connect loop (8 attempts × init_timeout + 120s margin). Before each attempt, check the wall clock; if exceeded, raise OSError immediately instead of attempting another initialize(). 2. **Fresh Application per retry**: On each failed attempt, rebuild via and re-register all handlers. The old app is best-effort shutdown with . This ensures each retry starts with a clean slate — no stale transports, no stale flag, no leaked state from the previous attempt. 3. **BaseException logging + propagation**: Added (placed LAST after all other handlers) to log CancelledError and other non-Exception signals before propagating. Previously these exited the retry loop silently with no log message. 4. ** block for app rebuild**: The clause runs after every failed attempt that isn't the last, rebuilding the app and discarding the old one regardless of which exception class caused the failure.
Collaborator
|
Merged via #70884. Your commits cherry-picked with authorship preserved. Note: the commits were authored under the "Hermes Agent" identity (agent@hermes.dev) — added a contributor email mapping so CI attribution passes. For future PRs, consider setting your git identity before committing so your commits carry your name. Thanks for the thorough root cause analysis on the connect hang — the three-bug breakdown (reused app, CancelledError propagation, missing total watchdog) was spot on. |
This was referenced Aug 3, 2026
Closed
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Fixes #67498 — Telegram gateway hangs forever at 'Connecting to Telegram (attempt 1/8)...' with all event loop threads idle in select(). The retry loop silently exits without logging attempt 2/8 or timeout.
Root Cause Analysis
Three bugs in the retry loop inside connect() (plugins/platforms/telegram/adapter.py):
1. Reused self._app across all 8 retry attempts
The Application object was built ONCE before the retry loop. After a failed initialize() the app could be in a partially-initialized state where:
2. CancelledError propagated silently
asyncio.CancelledError is a BaseException, not an Exception. None of the three except handlers caught it, so it propagated through the retry loop with zero logging — the task driving the loop could silently exit with no trace.
3. No total watchdog for the entire retry loop
Only per-attempt timeouts existed (via _await_with_thread_deadline, 30s default each). If the loop itself stalled between attempts (during asyncio.sleep(), cleanup, or scheduling), there was no mechanism to bound the total connect time.
Changes
Total watchdog deadline: Computes a total deadline (8 × init_timeout + 120s margin) before the loop. Each attempt checks the wall clock against this deadline and raises OSError immediately if exceeded — ensures the retry ladder always terminates.
Fresh Application per retry: On each failed attempt (signal via rebuild_app flag), the finally block rebuilds self._app via builder.build(), re-registers all handlers, and best-effort shuts down the old app. Each retry starts with a clean Application — no stale state leaking across attempts.
BaseException logging: Added except BaseException: (placed LAST after Exception, OSError, TimeoutError) that logs CancelledError and other non-Exception signals before propagating. Prevents silent task exit without diagnostics.
App rebuild in finally: The finally: clause runs after every failed attempt, ensuring the app is always rebuilt regardless of which exception class caused the failure.
Testing