Skip to content

fix(telegram): release timed-out spooled lanes#93040

Closed
vortexopenclaw wants to merge 2 commits into
openclaw:mainfrom
vortexopenclaw:fix/telegram-release-timed-out-spooled-lanes
Closed

fix(telegram): release timed-out spooled lanes#93040
vortexopenclaw wants to merge 2 commits into
openclaw:mainfrom
vortexopenclaw:fix/telegram-release-timed-out-spooled-lanes

Conversation

@vortexopenclaw

@vortexopenclaw vortexopenclaw commented Jun 14, 2026

Copy link
Copy Markdown
Contributor

Summary

  • Problem: Telegram isolated-ingress lanes can stay guarded indefinitely when a spooled update handler times out, the reply fence is aborted, and the handler still refuses to settle inside the abort-grace window. Later updates in the same Telegram chat/topic lane remain blocked until the whole gateway is restarted.
  • Solution: once the timed-out update has been marked failed and the reply fence has been superseded, release the active lane guard after the abort-grace window and restart isolated ingress so later same-lane updates can drain.
  • What changed: the timeout recovery path now removes the stuck handler from the active lane registry when it ignores abort, logs the release/restart behavior, and returns a restart request. The regression test now proves a later same-lane update drains even while the old handler promise remains unresolved.

AI-assisted: prepared with Codex.

Change Type

  • Bug fix
  • Feature
  • Refactor required for the fix
  • Docs
  • Security hardening
  • Chore/infra

Scope

  • Gateway / orchestration
  • Skills / tool execution
  • Auth / credentials
  • Memory / storage
  • Integrations
  • API / contracts
  • UI / DX
  • CI/CD / infra

Linked Issue/PR

Motivation

Telegram isolated ingress already detects spooled handler timeouts, marks the timed-out update failed, and aborts active reply work for the lane. However, if the handler ignores that abort, the lane remains present in activeSpooledUpdateHandlersByLane, so every later same-lane update stays blocked. The issue report includes a live repro where this persisted across provider auto-restarts for many hours and only cleared after a full gateway restart.

Real behavior proof

  • Behavior or issue addressed: a later Telegram update in the same lane should drain after the earlier spooled handler is timed out and failed, even if the old handler promise ignores reply abort and does not settle inside the grace window.
  • Real environment tested: local macOS source checkout.
  • Exact steps or command run after this patch:
node scripts/run-vitest.mjs extensions/telegram/src/polling-session.test.ts --reporter=verbose
  • Evidence after fix: copied terminal output from a local source-checkout run, showing the focused Telegram polling session shard passed with the new recovery case included.
Test Files  1 passed (1)
Tests       58 passed (58)
[test] passed 1 Vitest shard in 17.43s
  • Observed result after fix: the regression test fails update 42, releases the old lane guard after the 100 ms abort grace window, restarts isolated ingress, drains later same-lane update 43, and leaves no pending spooled updates.

  • What was not tested: a live Telegram Bot API outage/timeout. The covered behavior is the isolated-ingress lane registry and spool-drain recovery path with synthetic spooled updates.

  • Native Telegram proof attempt: Mantis Telegram Desktop Proof ran against baseline 6134c00657a24 and candidate f30bbd80ecbd, but the runner could not capture this recovery from visible chat input. Run: https://github.com/openclaw/openclaw/actions/runs/27507856635

  • Mantis evidence summary: the generated manifest says this recovery requires a timed-out Telegram reply handler that keeps running after abort, which the native proof runner cannot trigger from chat input.

  • Local re-check on June 14, 2026: reran node scripts/run-vitest.mjs extensions/telegram/src/polling-session.test.ts --reporter=verbose; result was Test Files 1 passed (1), Tests 58 passed (58), with the new releases a timed-out lane when the old handler ignores reply abort case passing.

Root Cause

  • Root cause: timeout recovery returned restart: false when the handler ignored abort, intentionally keeping the active handler registered as the lane guard.
  • Missing detection / guardrail: the existing test asserted that blocked behavior, so the suite preserved the lane stall rather than requiring recovery.

Regression Test Plan

  • Coverage level that should have caught this:
    • Unit test
    • Integration test
    • End-to-end test
    • Existing coverage already sufficient
  • Target test or file:
    • extensions/telegram/src/polling-session.test.ts
  • Scenario the test locks in:
    • A timed-out same-lane handler that ignores reply abort is failed and removed from the active lane registry after the abort-grace window.
    • Isolated ingress restarts and drains the later same-lane update instead of leaving it pending.

User-visible / Behavior Changes

  • Telegram DM/topic delivery can recover from a wedged spooled reply handler without requiring a full gateway restart.
  • The timed-out update is still marked failed before the lane is released.
  • Later same-lane updates are allowed to drain after the configured abort-grace window.

Security Impact

  • New permissions/capabilities? No
  • Auth/credential handling changed? No
  • New/changed network calls? No
  • Command/tool execution surface changed? No
  • Data access scope changed? No

Verification

git diff --check
node scripts/run-vitest.mjs extensions/telegram/src/polling-session.test.ts --reporter=verbose

Notes:

  • The focused Vitest command was run from a fresh git worktree with dependencies resolved through the existing hydrated local checkout.
  • The commit hook tried to run pnpm install inside the fresh worktree and aborted because there was no TTY for module purge confirmation, so the commit was made with hooks bypassed after the explicit focused test and git diff --check both passed.

@openclaw-barnacle openclaw-barnacle Bot added channel: telegram Channel integration: telegram size: XS triage: needs-real-behavior-proof Candidate: external PR needs after-fix proof from a real setup. labels Jun 14, 2026
@clawsweeper

clawsweeper Bot commented Jun 14, 2026

Copy link
Copy Markdown
Contributor

Codex review: needs real behavior proof before merge. Reviewed July 1, 2026, 4:50 PM ET / 20:50 UTC.

Summary
The current PR head changes Telegram polling-session timeout logging and regression coverage to keep an ignored-abort spooled handler lane guarded until the old handler settles.

PR surface: Source +3, Tests +9. Total +12 across 2 files.

Reproducibility: yes. at source level: current main and the current PR head keep update 43 pending when update 42 times out and the old same-lane handler ignores reply abort. I did not run live Telegram fault injection in this read-only review.

Review metrics: 1 noteworthy metric.

  • Proof/head mismatch: 1 stale proof block. The PR body and comment prove the earlier release-and-drain commit, while current head b14c78f expects the later same-lane update to remain pending.

Root-cause cluster
Relationship: fixed_by_candidate
Canonical: #91456
Summary: This PR is the linked candidate for the canonical Telegram guarded-lane timeout issue, but the latest head no longer fixes the recovery path.

Members:

Proposal only: this assessment does not dispatch repair, suppress jobs, mutate sibling items, close, or merge anything.

Merge readiness
Overall: 🧂 unranked krab
Proof: 🧂 unranked krab
Patch quality: 🧂 unranked krab
Result: blocked until stronger real behavior proof is added.

Overall follows the weaker of proof and patch quality, so missing proof can cap an otherwise strong patch.

Rank-up moves:

  • Rebase the branch and resolve the current conflict against main.
  • [P1] Implement the maintainer-requested hard cancellation or process-restart boundary, or retitle/close the branch so it no longer claims to fix the linked issue.
  • [P1] Add current-head redacted runtime logs, terminal output, or an artifact showing the timed-out update failed and the next same-lane update drained safely.

Proof guidance:

  • [P1] Needs stronger real behavior proof before merge: The supplied terminal proof is stale for the current head because it shows the earlier release-and-drain behavior, while the latest commit expects the later update to remain pending. After adding proof, update the PR body; ClawSweeper should re-review automatically. If it does not, the PR author or someone with repository write access can comment @clawsweeper re-review.

Risk before merge

  • [P1] Merging the current head would not fix the linked Telegram guarded-lane message-delivery failure because it still leaves later same-lane updates pending when the old handler ignores abort.
  • [P1] The supplied proof is stale for the current head: it describes the earlier release-and-drain behavior, while the latest commit asserts the later update remains pending.
  • [P1] The branch is currently conflicting against main, so even a product-approved direction still needs a rebase and renewed review against the current polling-session tests.
  • [P1] The persistent forum-topic black-hole report at Telegram: specific forum topic becomes permanent inbound black hole after stuck-session recovery #91564 overlaps the log signature but includes post-restart behavior that this PR should not be assumed to fix.

Maintainer options:

  1. Rework Around A Hard Stop Boundary (recommended)
    Before merge, rebase and make later same-lane updates drain only after the old handler has actually stopped, been killed, or been isolated by a process restart boundary.
  2. Own The Guarded-Lane Behavior Explicitly
    Maintainers can choose to preserve serialization over auto-recovery, but then the branch should be retitled or narrowed so it no longer claims to fix the linked message-delivery issue.
  3. Close And Replace The Branch
    If this branch is no longer the intended fix, close it and keep Telegram DM lane can remain guarded after send timeout, delaying or dropping direct messages #91456 open for a replacement recovery design with current-head proof.

Next step before merge

  • [P1] The remaining blocker is maintainer direction on the safe cancellation/restart contract plus a branch rebase, not a narrow automated repair on this conflicting PR head.

Security
Cleared: The diff only touches Telegram plugin runtime/test recovery logic and adds no dependency, workflow, permission, secret, package, or script surface.

Review findings

  • [P1] Do not assert the next same-lane update remains pending — extensions/telegram/src/polling-session.test.ts:2862
Review details

Best possible solution:

Rebase and implement a maintainer-approved hard cancellation or process-restart boundary so a timed-out failed update cannot continue concurrently while later same-lane updates can eventually drain.

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

Yes at source level: current main and the current PR head keep update 43 pending when update 42 times out and the old same-lane handler ignores reply abort. I did not run live Telegram fault injection in this read-only review.

Is this the best way to solve the issue?

No for the current head. It codifies the existing blocked-lane outcome, while the earlier release-after-grace approach was already rejected without a hard cancellation or process-restart boundary.

Full review comments:

  • [P1] Do not assert the next same-lane update remains pending — extensions/telegram/src/polling-session.test.ts:2862
    At the current head, this assertion locks in update 43 staying queued after update 42 times out and ignores abort. That is the guarded-lane failure this branch is linked to fix, so the PR needs a hard cancellation or restart boundary before it can claim release-and-drain recovery.
    Confidence: 0.92

Overall correctness: patch is incorrect
Overall confidence: 0.88

AGENTS.md: found and applied where relevant.

Codex review notes: model internal, reasoning high; reviewed against 44e88f550b69.

Label changes

Label justifications:

  • P1: The PR is tied to a real Telegram DM/topic workflow where later inbound messages can remain blocked until process-level recovery.
  • merge-risk: 🚨 message-delivery: The current head preserves the condition where a later Telegram update remains pending behind a timed-out same-lane handler.
  • merge-risk: 🚨 session-state: The diff touches same-lane handler guard state while an old Telegram handler promise may still be unresolved.
  • rating: 🧂 unranked krab: Overall readiness is 🧂 unranked krab; proof is 🧂 unranked krab and patch quality is 🧂 unranked krab.
  • status: 📣 needs proof: The PR needs real behavior proof before ClawSweeper can clear the contributor ask. Needs stronger real behavior proof before merge: The supplied terminal proof is stale for the current head because it shows the earlier release-and-drain behavior, while the latest commit expects the later update to remain pending. After adding proof, update the PR body; ClawSweeper should re-review automatically. If it does not, the PR author or someone with repository write access can comment @clawsweeper re-review.
Evidence reviewed

PR surface:

Source +3, Tests +9. Total +12 across 2 files.

View PR surface stats
Area Files Added Removed Net
Source 1 5 2 +3
Tests 1 18 9 +9
Docs 0 0 0 0
Config 0 0 0 0
Generated 0 0 0 0
Other 0 0 0 0
Total 2 23 11 +12

What I checked:

  • Root policy read: Root AGENTS.md was read fully and applied because this PR touches Telegram message delivery and session-state recovery, which the policy treats as high-confidence review surfaces. (AGENTS.md:1, 44e88f550b69)
  • Scoped extension policy read: The scoped extensions policy was read and applied because the PR changes bundled Telegram plugin runtime and test code under extensions/telegram. (extensions/AGENTS.md:1, 44e88f550b69)
  • Telegram maintainer note: The Telegram maintainer note says Telegram behavior PRs touching transport, topics, callbacks, authorization, or reply context need real Telegram proof or an equivalent live probe over synthetic-only validation. (.agents/maintainer-notes/telegram.md:37, 44e88f550b69)
  • PR head keeps ignored-abort lane guarded: At the current PR head, the ignored-abort branch logs that the lane stays guarded until the handler stops and returns restart false instead of releasing the lane for the next same-lane update. (extensions/telegram/src/polling-session.ts:923, b14c78f55884)
  • PR head test locks in pending update: The changed test now expects update 43 to remain pending after update 42 times out and ignores reply abort, contradicting the release-and-drain behavior described in the PR body. (extensions/telegram/src/polling-session.test.ts:2862, b14c78f55884)
  • Current main still has the guarded branch: Current main also keeps the ignored-abort handler registered and returns restart false, so the central linked user problem is not implemented on main by this behavior. (extensions/telegram/src/polling-session.ts:1095, 44e88f550b69)

Likely related people:

  • 0xghost42: Authored the recent merged Telegram lone active spooled-handler timeout recovery that changed the same timeout branch and tests. (role: recent area contributor; confidence: high; commits: 11b6c01198a0; files: extensions/telegram/src/polling-session.ts, extensions/telegram/src/polling-session.test.ts)
  • joshavant: Authored major Telegram stalled isolated spool handler recovery and buffered replay work that shaped the timeout, reply-fence, and restart path. (role: feature-history owner; confidence: high; commits: b7735f88fa27, 9921825e1795; files: extensions/telegram/src/polling-session.ts, extensions/telegram/src/polling-session.test.ts, extensions/telegram/src/telegram-reply-fence.ts)
  • vincentkoc: Recently stabilized Telegram spool timeout recovery assertions and appears in adjacent release/history for the current Telegram polling-session path. (role: recent area contributor; confidence: medium; commits: 9de6a99c8f72, e085fa1a3ffd; files: extensions/telegram/src/polling-session.test.ts, extensions/telegram/src/polling-session.ts)
  • obviyus: Requested changes on the first lane-release commit and named the hard cancellation or process-restart boundary expected before same-lane updates can safely drain. (role: current reviewer; confidence: medium; commits: f30bbd80ecbd; files: extensions/telegram/src/polling-session.ts, extensions/telegram/src/polling-session.test.ts)
What the crustacean ranks mean
  • 🦀 challenger crab: rare, exceptional readiness with strong proof, clean implementation, and convincing validation.
  • 🦞 diamond lobster: very strong readiness with only minor maintainer review expected.
  • 🐚 platinum hermit: good normal PR, likely mergeable with ordinary maintainer review.
  • 🦐 gold shrimp: useful signal, but proof or patch confidence is still limited.
  • 🦪 silver shellfish: thin signal; proof, validation, or implementation needs work.
  • 🧂 unranked krab: not merge-ready because proof is missing/unusable or there are serious correctness or safety concerns.
  • 🌊 off-meta tidepool: rating does not apply to this item.

Shiny media proof means a screenshot, video, or linked artifact directly shows the changed behavior. Runtime, network, CSP, and security claims still need visible diagnostics.

How this review workflow works
  • ClawSweeper keeps one durable marker-backed review comment per issue or PR.
  • Re-runs edit this comment so the latest verdict, findings, and automation markers stay together instead of adding duplicate bot comments.
  • A fresh review can be triggered by eligible @clawsweeper re-review comments, exact-item GitHub events, scheduled/background review runs, or manual workflow dispatch.
  • PR/issue authors and users with repository write access can comment @clawsweeper re-review or @clawsweeper re-run on an open PR or issue to request a fresh review only.
  • Maintainers can also comment @clawsweeper review to request a fresh review only.
  • Fresh-review commands do not start repair, autofix, rebase, CI repair, or automerge.
  • Maintainer-only repair and merge flows require explicit commands such as @clawsweeper autofix, @clawsweeper automerge, @clawsweeper fix ci, or @clawsweeper address review.
  • Maintainers can comment @clawsweeper explain to ask for more context, or @clawsweeper stop to stop active automation.

@openclaw-barnacle openclaw-barnacle Bot added proof: supplied External PR includes structured after-fix real behavior proof. and removed triage: needs-real-behavior-proof Candidate: external PR needs after-fix proof from a real setup. labels Jun 14, 2026
@clawsweeper clawsweeper Bot added rating: 🦪 silver shellfish Thin PR readiness signal; proof, validation, or implementation needs work. status: 📣 needs proof The PR needs real behavior proof before ClawSweeper can clear the contributor ask. P1 High-priority user-facing bug, regression, or broken workflow. merge-risk: 🚨 message-delivery 🚨 May drop, duplicate, misroute, suppress, or wrongly target messages. merge-risk: 🚨 session-state 🚨 May lose, corrupt, stale, or mis-associate session, agent, or context state. labels Jun 14, 2026
@vortexopenclaw

Copy link
Copy Markdown
Contributor Author

@clawsweeper re-review

@clawsweeper

clawsweeper Bot commented Jun 14, 2026

Copy link
Copy Markdown
Contributor

🦞🧹
ClawSweeper re-review requested.

I asked ClawSweeper to review this item again.
Action: item re-review queued (workflow sweep.yml, event repository_dispatch).
Result: the existing ClawSweeper review comment will be edited in place when the review finishes.

Re-review progress:

@vortexopenclaw

vortexopenclaw commented Jun 14, 2026

Copy link
Copy Markdown
Contributor Author

Proof update: I tried the native Telegram proof path, but this specific recovery cannot be triggered by the current visible-chat Mantis runner.

What ran:

Result:

  • The workflow resolved baseline 6134c00657a24 and candidate f30bbd80ecbd.
  • It did not capture visual before/after proof.
  • The generated manifest says this recovery requires a timed-out Telegram reply handler that keeps running after abort, which the native proof runner cannot trigger from chat input.

I also reran the focused production-path regression locally after updating the PR body:

node scripts/run-vitest.mjs extensions/telegram/src/polling-session.test.ts --reporter=verbose
Test Files  1 passed (1)
Tests       58 passed (58)

The specific passing case is releases a timed-out lane when the old handler ignores reply abort: it fails the timed-out update, releases the same-lane guard after abort grace, restarts isolated ingress, drains the later same-lane update, and leaves no pending spooled updates.

Given the native runner limitation, I think this PR needs maintainer acceptance of the focused production-path synthetic proof, or an explicit proof-gate override, rather than another rerun of the same Mantis path.

@vortexopenclaw

Copy link
Copy Markdown
Contributor Author

@clawsweeper re-review

@obviyus obviyus 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.

Thanks for pushing on this. I do not think this is safe to land in its current shape.

The patch releases the Telegram lane after the timeout grace even when the old same-lane handler is still running. That fixes the permanent stuck-lane symptom, but it can let update N and update N+1 execute concurrently in the same DM/topic lane if the old handler ignores abort. That breaks the serialization invariant and can produce late replies, duplicate/out-of-order sends, or stale session writes.

What I would expect from a root-cause fix: fail the timed-out update, abort reply work, then establish a hard cancellation boundary for the old handler before releasing the lane. In practice that means making the handler execution killable or forcing a clean gateway/process restart boundary, so later same-lane updates only drain after the previous handler is actually dead. A TTL-style lane release is a mitigation, not the root fix.

@clawsweeper clawsweeper Bot added rating: 🧂 unranked krab Not merge-ready due to missing proof or serious correctness/safety concerns. and removed rating: 🦪 silver shellfish Thin PR readiness signal; proof, validation, or implementation needs work. labels Jul 1, 2026
@vortexopenclaw

Copy link
Copy Markdown
Contributor Author

Closing this branch.

I compared the current head (b14c78f558) against main's #recoverTimedOutSpooledHandler: the ignored-abort branch now returns restart: false and keeps the lane guarded, identical to main's existing behavior. The first commit on this branch (f30bbd80ec) did release the lane after abort-grace and restart isolated ingress, but the second commit reverted that release path back to "keep guarded" after reviewer pushback that a log-based release without a hard cancellation/process-restart boundary was unsafe.

So the current head is a no-op relative to main and does not fix #91456. Per ClawSweeper's review, the real fix needs a hard cancellation or process-restart boundary that can actually stop a wedged handler task, not just abort its reply fence — that's a design decision (how to safely kill/isolate an in-flight handler without corrupting spool/session state) rather than something to bolt on via rebase.

Closing this PR rather than leaving a stale/no-op branch open. Leaving #91456 open for a follow-up design that implements an actual hard-stop boundary.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

channel: telegram Channel integration: telegram merge-risk: 🚨 message-delivery 🚨 May drop, duplicate, misroute, suppress, or wrongly target messages. merge-risk: 🚨 session-state 🚨 May lose, corrupt, stale, or mis-associate session, agent, or context state. P1 High-priority user-facing bug, regression, or broken workflow. proof: supplied External PR includes structured after-fix real behavior proof. rating: 🧂 unranked krab Not merge-ready due to missing proof or serious correctness/safety concerns. size: XS status: 📣 needs proof The PR needs real behavior proof before ClawSweeper can clear the contributor ask.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Telegram DM lane can remain guarded after send timeout, delaying or dropping direct messages

2 participants