Skip to content

fix(telegram): drain ingress with native queue claims#96912

Closed
RomneyDa wants to merge 4 commits into
mainfrom
fix/telegram-sqlite-stale-claims
Closed

fix(telegram): drain ingress with native queue claims#96912
RomneyDa wants to merge 4 commits into
mainfrom
fix/telegram-sqlite-stale-claims

Conversation

@RomneyDa

@RomneyDa RomneyDa commented Jun 26, 2026 •
edited
Loading

Copy link
Copy Markdown
Member

What Problem This Solves

Fixes #90945.

Telegram isolated ingress stores Bot API updates in the shared SQLite channel_ingress_events queue. The deadlock had two parts: Telegram selected pending rows outside the queue's native claim flow, and stale claimed rows could survive a restart when the recorded owner PID was reused by the new process. In containers this commonly means PID 1, so a dead claim could look live forever and block later same-lane updates.

This branch is rebased on the merged #96962 claim-refresh fix. It keeps that liveness/lease behavior and adds the separate queue-selection/PID-reuse fix.

Why This Change Was Made

Telegram now asks the shared ingress queue to atomically claimNext the next eligible update, using the queue's existing claim token for complete/release/fail operations. The queue selection skips blocked lanes while preserving Telegram update-id ordering.

The queue claim path also blocks lanes from claimed rows in the same candidate snapshot. That preserves the old per-update ordering invariant if a sibling drainer claims an earlier same-lane row between Telegram's pending snapshot and claimNext.

The stale claimed-row fix stays in Telegram's existing drain recovery path: a claim with this process's PID but a different Telegram process owner id is no longer treated as another live process. The normal stale-claim recovery releases that row back to pending, so the original inbound update is handled before later same-lane updates instead of being failed or skipped.

This keeps recovery local to Telegram's drain loop instead of adding a gateway startup sweep.

User Impact

Telegram users should see stuck isolated-ingress updates drain again without manual SQLite edits. Restarted or recovered Telegram sessions can release PID-reused stale claims, reclaim the original update, skip genuinely live blocked lanes, preserve active claim refresh from #96962, and continue unrelated lanes.

Evidence

  • node scripts/run-vitest.mjs extensions/telegram/src/telegram-ingress-spool.test.ts extensions/telegram/src/polling-session.test.ts src/channels/message/ingress-queue.test.ts (81 Telegram tests, 16 queue tests passed)
  • node scripts/run-oxlint.mjs extensions/telegram/src/polling-session.ts extensions/telegram/src/telegram-ingress-spool.ts extensions/telegram/src/telegram-ingress-spool.test.ts extensions/telegram/src/polling-session.test.ts src/channels/message/ingress-queue.ts src/channels/message/ingress-queue.test.ts
  • pnpm exec oxfmt --check --threads=1 extensions/telegram/src/polling-session.ts extensions/telegram/src/telegram-ingress-spool.ts extensions/telegram/src/telegram-ingress-spool.test.ts extensions/telegram/src/polling-session.test.ts src/channels/message/ingress-queue.ts src/channels/message/ingress-queue.test.ts
  • git diff --check origin/main...HEAD
  • .agents/skills/autoreview/scripts/autoreview --mode branch --base origin/main (clean after fixing the concurrent same-lane claim race it found)

@openclaw-barnacle openclaw-barnacle Bot added gateway Gateway runtime size: M maintainer Maintainer-authored PR labels Jun 26, 2026
@clawsweeper

clawsweeper Bot commented Jun 26, 2026 •
edited
Loading

Copy link
Copy Markdown
Contributor

Codex review: needs maintainer review before merge. Reviewed June 26, 2026, 5:25 PM ET / 21:25 UTC.

Summary
The branch changes Telegram isolated ingress to claim queued updates through ChannelIngressQueue.claimNext, recover PID-reused stale claims, and add queue/Telegram regression coverage.

PR surface: Source +68, Tests +228. Total +296 across 6 files.

Reproducibility: yes. at source level. Current main preserves the PID-reuse live-owner predicate and the linked issue reports durable Telegram claimed/pending backlogs; I did not run a live Telegram replay in this read-only review.

Review metrics: 1 noteworthy metric.

  • Queue Selector API: 4 added options. orderBy, scanLimit, candidateIds, and deriveLaneKey expand an SDK-exported queue method, so maintainers should notice the contract change before merge.

Root-cause cluster
Relationship: fixed_by_candidate
Canonical: #90945
Summary: This PR is a candidate fix for the canonical Telegram SQLite stale ingress-claim deadlock, with related PRs covering narrower predicate recovery or broader startup sweep recovery.

Members:

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

Merge readiness
Overall: 🦐 gold shrimp
Proof: 🦐 gold shrimp
Patch quality: 🐚 platinum hermit
Result: ready for maintainer review.

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

Rank-up moves:

  • [P2] Get explicit maintainer acceptance for the SDK selector options or internalize them before merge.
  • Decide whether Telegram-drain scoped recovery should land separately from the broader startup-sweep PR.
  • Capture Telegram Desktop or live Telegram proof if maintainers want visible end-to-end recovery evidence.

Mantis proof suggestion
Native Telegram proof would materially show whether stale-claim recovery restores visible chat delivery and preserves queued update order. A maintainer can ask Mantis to capture proof by posting this exact PR comment:

@openclaw-mantis telegram desktop proof: verify a PID-reused stale Telegram ingress claim is recovered and queued messages drain in update order.

Risk before merge

Maintainer options:

  1. Accept SDK Queue Selectors
    Maintainers can explicitly accept the four new claimNext selector options as supported plugin-facing queue API before merge.
  2. Internalize Selector Logic
    Move the new selector behavior behind a private Telegram/core helper if maintainers do not want these options to become SDK contract.
  3. Defer To Startup Sweep
    Pause this PR if maintainers want the broader stale SQLite ingress claim startup sweep to be the only canonical recovery path.

Next step before merge

  • [P2] Manual review is required because the PR is draft, protected by the maintainer label, and needs maintainer acceptance of the SDK selector API and recovery scope rather than a narrow automated repair.

Security
Cleared: The diff changes local TypeScript queue/session logic and tests; it adds no dependencies, workflows, secrets handling, downloads, or permission changes.

Review details

Best possible solution:

Merge only after maintainers explicitly accept the queue selector API and Telegram-scoped recovery, or revise the selectors behind a private seam while keeping broader startup recovery tracked separately.

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

Yes, at source level. Current main preserves the PID-reuse live-owner predicate and the linked issue reports durable Telegram claimed/pending backlogs; I did not run a live Telegram replay in this read-only review.

Is this the best way to solve the issue?

Mostly yes. Queue-native claiming is a maintainable fix shape for the Telegram drain path, but the SDK API expansion and scope versus broader startup recovery need maintainer acceptance.

AGENTS.md: found and applied where relevant.

Codex review notes: model internal, reasoning high; reviewed against 29680046801d.

Label changes

Label justifications:

  • P1: The PR targets a Telegram ingress deadlock where accepted inbound messages can remain undelivered until manual SQLite repair.
  • merge-risk: 🚨 compatibility: The diff expands an SDK-exported queue method with new selector options that external plugins could observe or depend on.
  • merge-risk: 🚨 message-delivery: The diff changes how Telegram pending and claimed ingress rows are selected, recovered, and drained for delivery.
  • merge-risk: 🚨 session-state: The change mutates durable channel ingress claim and lane state that gates Telegram session progress.
  • rating: 🦐 gold shrimp: Overall readiness is 🦐 gold shrimp; proof is 🦐 gold shrimp and patch quality is 🐚 platinum hermit.
  • status: 👀 ready for maintainer look: ClawSweeper has no concrete contributor-facing blocker left for this PR. Not applicable: The external contributor proof gate is not applicable because this is a MEMBER PR; the body supplies focused tests/checks but no live Telegram artifact.
  • mantis: telegram-visible-proof: Mantis should capture Telegram visible proof. The PR changes visible Telegram inbound delivery recovery, so a short Telegram Desktop proof can show stale-claim recovery and ordered queued-message drain.
Evidence reviewed

PR surface:

Source +68, Tests +228. Total +296 across 6 files.

View PR surface stats
Area Files Added Removed Net
Source 3 160 92 +68
Tests 3 245 17 +228
Docs 0 0 0 0
Config 0 0 0 0
Generated 0 0 0 0
Other 0 0 0 0
Total 6 405 109 +296

What I checked:

Likely related people:

  • 849261680: Recent current-main blame carries the central Telegram drain and queue code through commit 7fefc5f, and the same area is covered by the broader open startup-sweep PR for the canonical issue. (role: recent area contributor and related candidate-fix author; confidence: high; commits: 7fefc5ff58de, e30ce44cbead; files: extensions/telegram/src/polling-session.ts, extensions/telegram/src/telegram-ingress-spool.ts, src/channels/message/ingress-queue.ts)
  • joshavant: Merged PR Fix Telegram spooled claim refresh #96962 changed active Telegram spooled claim refresh and shared queue claim lifecycle immediately adjacent to this PR. (role: recent adjacent contributor; confidence: high; commits: db255b1154c1, 9921825e1795; files: extensions/telegram/src/polling-session.ts, extensions/telegram/src/telegram-ingress-spool.ts, src/channels/message/ingress-queue.ts)
  • vincentkoc: Commit 4957286 preserved live Telegram spool claim owners and is directly relevant to the current-PID/different-owner invariant this PR changes. (role: recent owner-liveness contributor; confidence: high; commits: 49572863d3c1, 2e75d925ad0a; files: extensions/telegram/src/telegram-ingress-spool.ts, extensions/telegram/src/telegram-ingress-spool.test.ts)
  • VACInc: Recent Telegram retry-delay work touches the same polling drain lane-blocking surface this PR rewrites. (role: recent adjacent contributor; confidence: medium; commits: 13ecca5408cb; files: extensions/telegram/src/polling-session.ts, extensions/telegram/src/telegram-ingress-spool.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.

@clawsweeper clawsweeper Bot added rating: 🧂 unranked krab Not merge-ready due to missing proof or serious correctness/safety concerns. status: ⏳ waiting on author ClawSweeper has contributor-facing work open and is waiting for author action. 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. mantis: telegram-visible-proof Mantis should capture Telegram visible proof. labels Jun 26, 2026
@clawsweeper
clawsweeper Bot temporarily deployed to qa-live-shared June 26, 2026 03:17 Inactive
@RomneyDa
RomneyDa marked this pull request as draft June 26, 2026 04:17
@openclaw-barnacle openclaw-barnacle Bot added channel: telegram Channel integration: telegram and removed gateway Gateway runtime labels Jun 26, 2026
@RomneyDa RomneyDa changed the title fix(channels): recover stale ingress claims on startup fix(telegram): drain ingress with native queue claims Jun 26, 2026
@RomneyDa

RomneyDa commented Jun 26, 2026

Copy link
Copy Markdown
Member Author

@clawsweeper re-review

@clawsweeper

clawsweeper Bot commented Jun 26, 2026 •
edited
Loading

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.

@RomneyDa
RomneyDa marked this pull request as ready for review June 26, 2026 04:32
@RomneyDa

RomneyDa commented Jun 26, 2026

Copy link
Copy Markdown
Member Author

@clawsweeper re-review

@clawsweeper clawsweeper Bot added the merge-risk: 🚨 compatibility 🚨 May break existing users, config, migrations, defaults, or upgrade paths. label Jun 26, 2026
@github-actions github-actions Bot mentioned this pull request Jun 26, 2026
Open
@RomneyDa

RomneyDa commented Jun 26, 2026

Copy link
Copy Markdown
Member Author

@clawsweeper re-review

@clawsweeper

clawsweeper Bot commented Jun 26, 2026 •
edited
Loading

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.

@clawsweeper clawsweeper Bot added rating: 🦐 gold shrimp Decent PR readiness signal, but merge confidence is limited. status: 👀 ready for maintainer look ClawSweeper has no concrete contributor-facing blocker left for this PR. and removed rating: 🧂 unranked krab Not merge-ready due to missing proof or serious correctness/safety concerns. status: ⏳ waiting on author ClawSweeper has contributor-facing work open and is waiting for author action. labels Jun 26, 2026
@RomneyDa

RomneyDa commented Jun 26, 2026

Copy link
Copy Markdown
Member Author

@openclaw-mantis telegram desktop proof: verify a PID-reused stale Telegram ingress claim is recovered and queued messages drain in update order.

@RomneyDa

RomneyDa commented Jun 26, 2026

Copy link
Copy Markdown
Member Author

Clarifying the recovery scope after the latest changes:

This is intentionally Telegram-scoped for recovery because Telegram is the only current production user of the queue claim/drain flow. The shared queue changes are generic and reusable for future channels; WhatsApp uses the same SQLite queue today, but only as an accept/complete/release journal, so there is no equivalent stale claim drain path to recover there yet.

So this is not a global startup sweep, but it does fix the Telegram claimed-row PID-reuse deadlock in the path that currently owns that behavior.

@RomneyDa
RomneyDa force-pushed the fix/telegram-sqlite-stale-claims branch from 1526324 to 58e228b Compare June 26, 2026 19:18
@RomneyDa
RomneyDa marked this pull request as draft June 26, 2026 21:20
@vincentkoc vincentkoc self-assigned this Jun 26, 2026
@clawsweeper clawsweeper Bot mentioned this pull request Jun 26, 2026
Merged
@vincentkoc

vincentkoc commented Jun 26, 2026

Copy link
Copy Markdown
Member

Superseded by the consolidated canonical fix in #97118.

#97118 now contains this PR's four queue-native claim/PID-reuse commits, preserves Dallin Romney's authorship, and adds the stalled-drain heartbeat/lease fix from the companion implementation. Keeping both PRs open would split one Telegram delivery fix across two competing drain paths.

The combined branch passed 99 focused tests, Testbox-through-Crabbox check:changed on tbx_01kw33f6bjngpjv0z5c1vtxzqv, local format/lint, and clean autoreview. Closing this draft so #97118 is the single review surface.

@vincentkoc vincentkoc closed this Jun 26, 2026
This was referenced Jun 28, 2026
Merged
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

No reviews

Assignees

@vincentkoc vincentkoc

Labels

channel: telegram Channel integration: telegram maintainer Maintainer-authored PR mantis: telegram-visible-proof Mantis should capture Telegram visible proof. merge-risk: 🚨 compatibility 🚨 May break existing users, config, migrations, defaults, or upgrade paths. 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. rating: 🦐 gold shrimp Decent PR readiness signal, but merge confidence is limited. size: L status: 👀 ready for maintainer look ClawSweeper has no concrete contributor-facing blocker left for this PR.

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

channel_ingress_events (SQLite): stale claims never recovered after session crash — Telegram DM deadlocks

2 participants

@RomneyDa @vincentkoc