Skip to content

fix(telegram): isolate getUpdates polling transport - #29326

Open
candyapplecorn wants to merge 1 commit into
NousResearch:mainfrom
candyapplecorn:fix/telegram-polling-transport
Open

candyapplecorn wants to merge 1 commit into
NousResearch:mainfrom
candyapplecorn:fix/telegram-polling-transport

Conversation

@candyapplecorn

@candyapplecorn candyapplecorn commented May 20, 2026

Copy link
Copy Markdown

Summary

  • isolate Telegram getUpdates polling onto an HTTPX request client with keep-alive disabled
  • add opt-in HERMES_TELEGRAM_POLL_TIMEOUT and HERMES_TELEGRAM_POLL_INTERVAL controls
  • leave PTB polling timing defaults unchanged unless those env vars are set
  • add regression tests covering polling transport isolation and env var clamping

Background

Repeated Telegram 409 Conflict errors were observed on a macOS launchd Hermes gateway with no visible second gateway process and no webhook configured. Raw/PTB repros still hit conflicts with Hermes stopped, while polling with fresh/no-keepalive transport and short polling stayed clean in the test window.

This PR keeps the broadly applicable transport isolation as the default change, while making polling timing explicit opt-in configuration for affected deployments.

Testing

  • python3 -m py_compile gateway/platforms/telegram.py
  • venv/bin/python -m pytest tests/gateway/test_telegram_polling_transport.py -q on the Mac mini checkout: 2 passed
  • Local checkout could not run pytest because this machine has no pytest installed: /usr/local/opt/python@3.14/bin/python3.14: No module named pytest

Refs: #29325

@candyapplecorn
candyapplecorn force-pushed the fix/telegram-polling-transport branch from 98f179a to 2c25c59 Compare May 20, 2026 14:10
@alt-glitch alt-glitch added type/bug Something isn't working P2 Medium — degraded but workaround exists comp/gateway Gateway runner, session dispatch, delivery platform/telegram Telegram bot adapter labels May 20, 2026
@IgorGanapolsky

Copy link
Copy Markdown
Contributor

I reproduced the same failure class on macOS launchd while debugging a Telegram gateway that had no visible duplicate local Hermes process.

Evidence from the local reproduction:

  • one local gateway process was running under launchd before shutdown/restart testing
  • getWebhookInfo returned an empty webhook URL and pending_update_count=0
  • conflicts still appeared as Telegram polling conflict ... terminated by other getUpdates request
  • after stopping Hermes for more than two minutes, raw getUpdates?timeout=1&limit=1 still returned a 409 once in 20 attempts
  • disabling fallback IPs and lowering poll timeout/interval reduced but did not eliminate the failure locally

This PR is the right consolidation point for the getUpdates transport mitigation. I independently opened #46996 before seeing this, but it duplicates the core keepalive isolation approach here, so I am closing that duplicate in favor of this PR and keeping the reproduction evidence here.

@teknium1

Copy link
Copy Markdown
Collaborator

Thanks for isolating the polling transport and including targeted coverage.

Problems

  • In the fallback-IP branch, this diff places limits beside a custom TelegramFallbackTransport. Current main records that HTTPX ignores client-level limits in that shape; the working implementation forwards limits into the transport itself (plugins/platforms/telegram/adapter.py:3139-3163, commit 01ee312de). The proposed no-keepalive behavior would therefore not apply on that branch.
  • The new HERMES_TELEGRAM_POLL_TIMEOUT and HERMES_TELEGRAM_POLL_INTERVAL controls are user-facing behavioral environment variables. Repository policy requires timeout/threshold configuration to use config.yaml (AGENTS.md:102-107).

Suggested changes

  • Re-scope to the transport mitigation and remove the polling timing env vars; use config plumbing only if a timing control remains necessary.
  • Port the work to plugins/platforms/telegram/adapter.py and test the fallback transport's inner HTTPX limits, as current coverage does in tests/gateway/test_telegram_network.py:357-386.

Automated hermes-sweeper review.

@teknium1 teknium1 added sweeper:risk-message-delivery Sweeper risk: may drop, duplicate, misroute, or suppress messages sweeper:risk-compatibility Sweeper risk: may break existing users, config, migrations, defaults, or upgrades sweeper:blast-moderate Sweeper blast radius: moderate — a subsystem or single platform labels Jul 13, 2026

@GottZ GottZ left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

This was generated by AI during triage.

Summary

Two PRs address the reported Telegram polling-conflict class by isolating the getUpdates HTTPX client and disabling its keep-alive, while leaving the general Bot API client unchanged. Both diffs also add polling-timing environment controls, but neither correctly applies the limits inside the custom fallback transport, and those user-facing controls conflict with the repository's config.yaml policy.

Related pull requests

  • #29326 related — (+127/-3) — revise before merge: The diff directly targets the suspected stale getUpdates-connection cause and adds focused coverage, but the keep_open review on #29326 identifies two blocking gaps visible in the diff: limits are passed beside TelegramFallbackTransport rather than into it, and timeout/interval are exposed through environment variables instead of config.yaml; the patch must also be ported to the current plugins/platforms/telegram/adapter.py implementation.
  • #46996 [closed] duplicate — (+106/-3) — closed duplicate, retained as corroborating implementation and test evidence: It independently implements essentially the same getUpdates-only no-keepalive mitigation and polling controls as #29326, including the same ineffective placement of limits beside TelegramFallbackTransport; its author closed it in favor of #29326 and moved the reproduction evidence there.

Duplicates

#46996 substantially duplicates #29326's getUpdates transport isolation, fallback/proxy/direct branches, polling timing controls, and regression coverage.

Suggested consolidation

Consolidate on #29326, but do not merge it as currently written: address the contributor keep_open review by moving the change to plugins/platforms/telegram/adapter.py, forwarding no-keepalive limits into the fallback transport itself, removing the environment-based timing controls or plumbing them through config.yaml, and covering the fallback path. Keep #46996 closed as a duplicate of #29326.

Complex graph

flowchart LR
    classDef open fill:#dbeafe,stroke:#1d4ed8,color:#1e3a8a
    classDef merged fill:#dcfce7,stroke:#15803d,color:#14532d
    classDef closed fill:#e5e7eb,stroke:#6b7280,color:#1f2937
    classDef unverified fill:#f3f4f6,stroke:#9ca3af,color:#374151
    classDef best stroke-width:3px,stroke:#b45309
    classDef target stroke-width:3px,stroke:#4338ca
    subgraph Dup29326 ["PRs duplicating each other"]
        P29326["PR #29326 (open)"]
        P46996["PR #46996 (closed)"]
    end
    class P29326 open
    class P46996 closed
    class P29326 target
    click P29326 "https://github.com/NousResearch/hermes-agent/pull/29326"
    click P46996 "https://github.com/NousResearch/hermes-agent/pull/46996"
Loading

Graph: solid arrow = fixes / best fix, dashed arrow = partial or unverified (see edge label); boxed group = PRs duplicating each other; amber border = best fix; indigo border = target; gray node = closed or no verify verdict yet (state tag in the node label).

Cross-PR triage: Reviewed 2 pull requests and 0 issues in this complex. Each diff was read against this issue; Assessment working set: 15 kB of PR diffs, 3 kB of issue/PR text, 3 kB of discussion (4 comments), 1 verify verdict. verdicts reflect diff content, not PR titles. Part of an automated triage batch.

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 P2 Medium — degraded but workaround exists platform/telegram Telegram bot adapter sweeper:blast-moderate Sweeper blast radius: moderate — a subsystem or single platform sweeper:risk-compatibility Sweeper risk: may break existing users, config, migrations, defaults, or upgrades 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.

5 participants