Skip to content

fix(telegram): drain outbound queue after polling reconnect#82227

Merged
steipete merged 2 commits into
mainfrom
fix/telegram-reconnect-delivery-drain
May 15, 2026
Merged

fix(telegram): drain outbound queue after polling reconnect#82227
steipete merged 2 commits into
mainfrom
fix/telegram-reconnect-delivery-drain

Conversation

@steipete

@steipete steipete commented May 15, 2026 •
edited
Loading

Copy link
Copy Markdown
Contributor

Summary

  • Drain Telegram queued outbound deliveries after polling confirms fresh getUpdates activity.
  • Scope reconnect recovery to the current Telegram account and reuse the existing delivery-queue reconnect drain.
  • Add regression coverage for normal polling and isolated ingress reconnect signals.

Problem

#50040 reports Telegram outbound reliability failures where network or stale-socket recovery can leave failed outbound replies in the durable delivery queue until a full gateway startup recovery happens. Current main already has Telegram transport dirty-rebuild/polling-stall recovery and the generic reconnect drain used by WhatsApp, but Telegram did not call that drain after polling recovered.

#82175 is related because stale-socket health recovery is the path that can restart Telegram polling without a full gateway restart. This PR does not claim to solve every health-state semantic in #82175; it closes the missing outbound recovery step once Telegram polling proves fresh transport liveness.

#82194 is separate: it is a direct-chat context dedupe PR for #82040, not a stale-socket or outbound delivery recovery fix.

Fixes #50040.
Refs #82175.
Refs #82194.

Real behavior proof

Behavior addressed: Telegram polling reconnect now schedules delivery-queue recovery after successful getUpdates liveness, including isolated ingress poll success, and repeats on later poll successes so same-cycle failures are not stranded.
Real environment tested: Mantis Telegram Live on branch fix/telegram-reconnect-delivery-drain, first candidate SHA 3b7f3dbd6b6e7db5c2da4efcd11348d5469145c2, using Convex-leased Telegram credentials and a Crabbox desktop transcript capture; current head SHA 258190d81f48b48d82ddad96ec93b6835aabbcaf adds the backoff-safety fixup verified by focused tests and Codex review.
Exact steps or command run after this patch: GitHub Actions workflow Mantis Telegram Live, run https://github.com/openclaw/openclaw/actions/runs/25930493495, scenario telegram-status-command; local node scripts/run-vitest.mjs extensions/telegram/src/polling-session.test.ts src/infra/outbound/delivery-queue.reconnect-drain.test.ts extensions/telegram/src/network-errors.test.ts.
Evidence after fix: Live Telegram artifacts from run https://github.com/openclaw/openclaw/actions/runs/25930493495 include screenshot/video/transcript files and telegram-qa-summary.json; the summary reports telegram-canary pass with reply matched in 1595ms and telegram-status-command pass with reply matched in 3604ms. The workflow conclusion is failed only after artifact upload because PR auto-comment token creation returned Invalid keyData.
Observed result after fix: Real Telegram bot replied to canary and /status on the branch SHA; focused tests confirm reconnect drain scheduling, account scoping, backoff-safe ambiguous send-error handling, isolated ingress poll-success drain, and repeated drain scheduling after later successful getUpdates calls.
What was not tested: forced live stale-socket wedge / real Telegram pending-outbound replay injection on this branch.

Codex review: /Users/steipete/Projects/agent-scripts/skills/codex-review/scripts/codex-review --parallel-tests "node scripts/run-vitest.mjs extensions/telegram/src/polling-session.test.ts src/infra/outbound/delivery-queue.reconnect-drain.test.ts extensions/telegram/src/network-errors.test.ts" returned clean after accepted findings were fixed.

@openclaw-barnacle openclaw-barnacle Bot added channel: telegram Channel integration: telegram size: M maintainer Maintainer-authored PR labels May 15, 2026
@clawsweeper

clawsweeper Bot commented May 15, 2026 •
edited
Loading

Copy link
Copy Markdown
Contributor

Codex review: needs real behavior proof before merge.

Summary
This PR imports the delivery-queue reconnect drain into Telegram polling, invokes it on getUpdates and isolated ingress poll success, adds regression tests, and updates the changelog.

Reproducibility: yes. at source level: current main's Telegram poll-success path only notes status and never invokes the shared reconnect drain. I did not establish a live stale-socket pending-delivery replay in this read-only pass.

Real behavior proof
Needs stronger real behavior proof before merge: The PR links live Telegram canary/status artifacts, but they do not show the changed reconnect or queued-pending replay path; add redacted logs, terminal output, video, or linked artifacts and update the PR body for re-review.

Next step before merge
Manual review is needed because the PR has the protected maintainer label, still needs changed-path live Telegram proof, and has a narrow duplicate-safe retry blocker.

Security
Cleared: The diff does not change dependencies, workflows, permissions, secrets handling, package resolution, or code execution surfaces.

Review findings

  • [P2] Keep reconnect drains duplicate-safe — extensions/telegram/src/polling-session.ts:55-58
Review details

Best possible solution:

Keep the reconnect drain, but make bypassBackoff depend only on duplicate-safe pre-connect evidence or leave backoff in place, then add redacted live proof that a queued Telegram delivery drains after reconnect.

Do we have a high-confidence way to reproduce the issue?

Yes, at source level: current main's Telegram poll-success path only notes status and never invokes the shared reconnect drain. I did not establish a live stale-socket pending-delivery replay in this read-only pass.

Is this the best way to solve the issue?

No, not yet. The reconnect hook is at the right ownership boundary, but the retry-bypass predicate should preserve Telegram's duplicate-safe send contract before this can merge.

Full review comments:

  • [P2] Keep reconnect drains duplicate-safe — extensions/telegram/src/polling-session.ts:55-58
    The reconnect drain bypasses delivery backoff when a stored Telegram send error broadly matches a network-send envelope. Existing Telegram send retry tests intentionally do not treat a plain Network request for 'sendMessage' failed! envelope as safe because Telegram may already have accepted the visible message, and drainPendingDeliveries will replay immediately when bypassBackoff is true.
    Confidence: 0.86

Overall correctness: patch is incorrect
Overall confidence: 0.86

Acceptance criteria:

  • node scripts/run-vitest.mjs extensions/telegram/src/polling-session.test.ts src/infra/outbound/delivery-queue.reconnect-drain.test.ts extensions/telegram/src/network-errors.test.ts
  • Live Telegram stale-socket or equivalent pending-outbound replay proof with tokens, phone numbers, IPs, and non-public endpoints redacted

What I checked:

  • PR diff adds a broad send-error backoff bypass: The PR adds shouldBypassReconnectBackoff and calls isRecoverableTelegramNetworkError with context send and allowMessageMatch true, so a plain stored Telegram send envelope can bypass reconnect retry backoff. (extensions/telegram/src/polling-session.ts:55, 3b7f3dbd6b6e)
  • Current main lacks the Telegram reconnect drain hook: Current main's TelegramPollingLivenessTracker onPollSuccess only updates polling status; it does not call the shared delivery-queue reconnect drain after getUpdates succeeds. (extensions/telegram/src/polling-session.ts:431, 7715b29aa231)
  • Existing send retry contract is duplicate-safe: Telegram tests intentionally reject broad send-context message matching, and isSafeToRetrySendError does not infer safety from a plain grammY send envelope. (extensions/telegram/src/network-errors.test.ts:249, 7715b29aa231)
  • Delivery drain obeys bypassBackoff: drainPendingDeliveries skips retry backoff when selectEntry returns bypassBackoff, so the PR's predicate changes retry timing for queued visible Telegram sends. (src/infra/outbound/delivery-queue-recovery.ts:544, 7715b29aa231)
  • Maintainer Telegram proof standard applies: The Telegram maintainer note requires real Telegram proof for transport-touching behavior PRs, preferably bot-to-bot QA or an equivalent live probe. (.agents/maintainer-notes/telegram.md:37, 7715b29aa231)
  • PR proof does not exercise the changed path: The PR body links a live Telegram canary/status workflow, but also says forced stale-socket or real pending-outbound replay was not tested on this branch. (3b7f3dbd6b6e)

Likely related people:

  • steipete: Authored or committed recent Telegram polling/surface refactors and is the committer on the stalled-polling transport rebuild commit adjacent to this reconnect path. (role: recent Telegram polling contributor; confidence: high; commits: 0c0f1e34cba0, b171e4211787, e035a0d98c91; files: extensions/telegram/src/polling-session.ts, extensions/telegram/src/monitor.ts)
  • sinogello: GitHub commit metadata lists sinogello as the author of the Telegram stalled-polling transport rebuild work that this PR builds on. (role: adjacent recovery contributor; confidence: high; commits: e035a0d98c91; files: extensions/telegram/src/polling-session.ts, extensions/telegram/src/monitor.ts)

Remaining risk / open question:

  • The PR can immediately replay ambiguous Telegram send failures because broad lastError matching bypasses reconnect backoff.
  • The supplied live proof does not exercise stale-socket recovery or real pending-outbound replay.
  • The protected maintainer label means this PR should stay open for explicit maintainer handling.

Codex review notes: model gpt-5.5, reasoning high; reviewed against 7715b29aa231.

Re-review progress:

@steipete
steipete force-pushed the fix/telegram-reconnect-delivery-drain branch from 3b7f3db to 258190d Compare May 15, 2026 17:44
@steipete
steipete merged commit 7270bb9 into main May 15, 2026
15 of 16 checks passed
@steipete
steipete deleted the fix/telegram-reconnect-delivery-drain branch May 15, 2026 17:46
@github-actions github-actions Bot mentioned this pull request May 15, 2026
Open
@amittell amittell mentioned this pull request May 17, 2026
Merged
2 tasks
github-actions Bot pushed a commit to Desicool/openclaw that referenced this pull request May 24, 2026
@steipete
fix(telegram): drain outbound queue after polling reconnect (openclaw…
65f6731 
…#82227)

* fix(telegram): drain outbound queue after polling reconnect

* fix(telegram): keep reconnect drain backoff-safe
jameslcowan pushed a commit to jameslcowan/openclaw that referenced this pull request Jun 2, 2026
@steipete
fix(telegram): drain outbound queue after polling reconnect (openclaw…
6058628 
…#82227)

* fix(telegram): drain outbound queue after polling reconnect

* fix(telegram): keep reconnect drain backoff-safe
SYU8384 pushed a commit to SYU8384/openclaw that referenced this pull request Jun 3, 2026
@steipete
fix(telegram): drain outbound queue after polling reconnect (openclaw…
8305623 
…#82227)

* fix(telegram): drain outbound queue after polling reconnect

* fix(telegram): keep reconnect drain backoff-safe
@clawsweeper clawsweeper Bot mentioned this pull request Jun 3, 2026
Open
sablehead pushed a commit to sablehead/openclaw that referenced this pull request Jun 10, 2026
@steipete
fix(telegram): drain outbound queue after polling reconnect (openclaw…
23a48b1 
…#82227)

* fix(telegram): drain outbound queue after polling reconnect

* fix(telegram): keep reconnect drain backoff-safe
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

No reviews

Assignees

No one assigned

Labels

channel: telegram Channel integration: telegram maintainer Maintainer-authored PR size: M

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

Telegram delivery reliability: polling stalls can lead to silent outbound message loss

1 participant

@steipete