fix(telegram): preserve forum topic routing - #76920
Conversation
|
Codex review: needs real behavior proof before merge. Reviewed July 31, 2026, 7:15 AM ET / 11:15 UTC. ClawSweeper reviewWhat this changesThe branch limits Telegram threadless retries, preserves topic IDs through draft and media sends, and carries explicit cron topic routes into later heartbeat and completion delivery. Merge readiness⛔ Blocked until stronger real behavior proof is added - 6 items remain Keep this PR open: it addresses a real Telegram cross-topic delivery problem and current Priority: P1 Review scores
Verification
How this fits togetherTelegram inbound topic metadata becomes an outbound flowchart LR
A[Telegram forum topic] --> B[Inbound session route]
B --> C[Telegram outbound send]
C --> D{Thread-not-found error?}
D --> E[General-topic recovery]
D --> F[Fail closed for explicit topic]
G[Cron announce target] --> H[Session delivery context]
H --> I[Heartbeat or completion route]
I --> C
Decision needed
Why: This is a deliberate delivery-scope tradeoff: fail-closed behavior avoids cross-topic/root-chat leakage, while the prior merged behavior preserves recovery for stale DM thread IDs. Tests cannot choose the supported compatibility contract. Before merge
Findings
Agent review detailsSecurityNone. PR surfaceSource +274, Tests +412. Total +686 across 12 files. View PR surface stats
Review metricsNone. Stored data modelPersistent data-model change detected: Mantis proof suggestionA current-head Telegram Desktop recording would directly validate that valid topic media stays in the requested topic and invalid explicit topics do not fall back to the root chat. A maintainer can ask Mantis to capture proof by posting this exact PR comment: Merge-risk optionsMaintainer options:
Technical reviewBest possible solution: First make the direct/private fallback policy explicit: either preserve the established DM recovery path or accept fail-closed routing as the supported contract. Then rebase to a clean current-main result and provide redacted current-head proof for topic media plus one end-to-end async route before merge. Do we have a high-confidence way to reproduce the issue? No high-confidence current-main live reproduction was established in this review. The supplied base-fails/head-passes test and older live Telegram transcript make the transport bug source-reproducible, but the current conflicted head still needs a refreshed real setup run. Is this the best way to solve the issue? Unclear. The forum fail-closed path is the narrowest way to prevent root-chat misdelivery, but changing direct/private fallback behavior from the merged recovery policy needs an explicit owner decision before it can be called the best solution. Full review comments:
Overall correctness: patch is incorrect AGENTS.md: found and applied where relevant. Codex review notes: model internal, reasoning high; reviewed against 22911713e511. LabelsLabel justifications:
EvidenceWhat I checked:
Likely related people:
Rank-up movesOptional improvements that raise the rating; they are not merge blockers.
Rating scale
Overall follows the weaker of proof and patch quality. Workflow
HistoryReview history (1 earlier review cycle)
|
|
Follow-up pushed in Addressed the prior review items:
Verification run locally:
|
This comment was marked as low quality.
This comment was marked as low quality.
d40225f to
54b7b0d
Compare
|
Rebased and refreshed this PR on current Current head: Updates in this refresh:
Local verification:
GitHub check status after push: mergeable, 88 checks passing, 20 skipped, 0 pending. The only failed check is still |
Test-based reproduction proofThis is automated reproduction evidence for the threadless-fallback boundary in this PR. It is not a live Telegram forum smoke, so it should not be treated as replacing the remaining Base reproduces the bug boundaryBase: I added a temporary failing regression test to a clean base worktree: pnpm test extensions/telegram/src/draft-stream.test.ts -- -t "reproduction: does not retry forum message preview sends without thread when thread is not found"Result: failed as expected. Base retries a forum/topic preview send without Key failure: That proves the pre-fix behavior still has a threadless retry path for explicit forum sends. PR head passes the fixed behaviorHead: The same behavior is covered by the committed PR test: pnpm test extensions/telegram/src/draft-stream.test.ts -- -t "does not retry forum message preview sends without thread when thread is not found"Result: So the current PR head proves base-fails/head-passes for the explicit forum send retry case: when a forum/topic send has a Residual: I still have not run a live Telegram forum bot smoke against this PR head. The local tests prove the fallback bug boundary; they do not prove end-to-end production Telegram delivery into a real forum topic. |
7fe0c05 to
bee6adf
Compare
|
@clawsweeper re-review Rebased this branch onto current
Local checks run after the update: I could not run local OpenGrep because |
|
🦞🧹 I asked ClawSweeper to review this item again. Re-review progress:
|
244d1d0 to
d7045b1
Compare
|
Heads up: this PR needs to be updated against current |
|
@clawsweeper re_review |
…back ClawSweeper P1: the media wrapper passed removeMessageThreadIdParam( plainMediaParams) even for the first attempt, so an HTML caption parse error for a topic-scoped media send retried without message_thread_id and could deliver topic media to the group root. Keep the thread for caption parse fallback; only strip it for the allowed thread-not-found retry.
|
@clawsweeper re_review |
|
@openclaw-mantis telegram desktop proof: verify current PR head keeps valid Telegram forum text/media sends in the requested topic and invalid explicit topics fail without a root-chat fallback. |
|
@kesslerio thanks for the PR. ClawSweeper is still waiting on real behavior proof before this can move forward. Useful proof can be a screenshot, short video, terminal output, copied live output, linked artifact, or redacted logs that show the changed behavior after the fix. Please redact private tokens, phone numbers, private endpoints, customer data, and anything else sensitive. Once proof is added to the PR body or a comment, ClawSweeper or a maintainer can re-check it. |
|
ClawSweeper status: review started. I am starting a fresh review of this pull request: fix(telegram): preserve forum topic routing This is item 1/1 in the current shard. Shard 40/44. This placeholder means the worker is alive and reading the current context. I will edit this same comment with the actual review when the claws are done clicking. Crustacean status: shell secured, claws on keyboard, evidence pebbles being sorted. |
|
Thanks for the careful work here. We retested current main at f2af4e9 with a real Telegram user in non-General forum topic 17. The inbound turn, typing state, and outbound reply all remained in topic 17, with one provider request and one reply. Since current main already has the intended behavior and we cannot reproduce the routing bug, this PR is no longer needed. Closing for now. |
Summary
message_thread_id=1), while failing closed for explicit non-General forum topic sends and direct/private topic sends.Change Type
Scope
Telegram extension outbound delivery, shared heartbeat target resolution, isolated cron session delivery metadata, and one test-only
memory-coremock compatibility update for the current subagent runtime contract.Linked Issue / PR
message_thread_idon outbound Telegram forum sends and was closed as duplicate/superseded by the same topic-routing work.message_thread_idpass-through now present onmain; this PR keeps the broader fail-closed and async-route preservation behavior.Root Cause
The Telegram sender retried
400: message thread not foundby removingmessage_thread_idtoo broadly. That is safe only for General-topic recovery, but unsafe for explicit non-General forum topics and direct/private topic sends because Telegram can then deliver into the wrong root context.A separate async path could reconstruct heartbeat/completion delivery from incomplete session metadata. Topic-scoped Telegram sessions and explicit cron announce jobs could lose the thread id before later automatic notices were sent.
Regression Test Plan
telegram:group:<id>:topic:<thread>fail closed.deliveryContextororigin.chatTypecarries the topic context.Real behavior proof
Behavior or issue addressed: Telegram topic sends must not silently retry without
message_thread_idafter Telegram reportsmessage thread not found; async heartbeat/completion delivery must preserve the intended topic route instead of falling back to a group/default route.Real environment tested: Disposable OpenClaw staging gateway on NixOS Linux, PR worktree
/tmp/openclaw-pr76920-merge, headf515e5d36ac1166b955838c21489390dbfd9dbe2, using@kesslerClawBotin a real Telegram forum group (-1003908474243). The staging gateway was started from this PR head, and the proof script used the PR's built Telegram runtime,sendMessageTelegramfromdist/extensions/telegram/runtime-api.js.Exact steps or command run after this patch: Started the disposable gateway with
OPENCLAW_CONFIG_PATH=/home/art/.local/state/openclaw-staging/disposable/openclaw.jsonandOPENCLAW_STATE_DIR=/home/art/.local/state/openclaw-staging/disposable/state, then ran a redacted proof script from/tmp/openclaw-pr76920-mergethat exercised: root chat-scoped send to-1003908474243, explicit General topic-1003908474243:topic:1, invalid explicit non-General topic-1003908474243:topic:999999, and valid explicit non-General topic-1003908474243:topic:16.Evidence after fix (screenshot, recording, terminal capture, console output, redacted runtime log, linked artifact, or copied live output): Copied live output from the staging gateway/proof run. Full PR comment: fix(telegram): preserve forum topic routing #76920 (comment)
Key copied output:
Observed result after fix: Root chat-scoped send succeeded without a topic/thread id; General topic delivery succeeded and remained chat-scoped; an invalid explicit non-General forum topic failed closed and did not retry threadless; a valid explicit non-General forum topic sent with
threadId=16, proving delivery stayed in the requested forum topic.What was not tested: I did not run a full live cron or heartbeat follow-up through the gateway in this proof pass. Existing focused tests still cover cron session delivery context and heartbeat topic-route reconstruction.
Proof limitations or environment constraints: The live proof covers the real Telegram transport and topic send path. Cron/heartbeat route reconstruction remains covered by focused regression tests rather than a scheduled end-to-end cron/heartbeat run.
Before evidence (optional but encouraged): ClawSweeper's current-main/source review for this PR identifies the pre-patch failure mode: explicit Telegram topic sends could retry threadless after
message thread not found, and async follow-up routing could lose topic context. The after-fix proof above shows the explicit send path no longer retries threadless for an invalid non-General topic.Verification
Current head
75acdc52684b07c8355966394f5ad5347a713e0awas refreshed against latestorigin/main, trimmed back to the intended nine-file effective diff, and pushed after the commands above passed locally.Security Impact
Reduces cross-topic message leakage in Telegram forum groups and private/direct topic contexts by failing closed when an explicit thread is stale or invalid, and by preserving topic context for automatic follow-up notices.
Compatibility / Migration
No config migration. Non-General forum topic and direct/private topic sends now surface the Telegram error instead of silently retrying threadless. Explicit cron announce routes now persist the intended delivery context for later automatic notices.
Risks
If an operator intentionally relied on stale explicit topic sends falling back to a root chat, those sends now fail. That behavior was the leakage bug this PR is meant to prevent.
The cron/session change is intentionally limited to resolved explicit delivery targets; bare
delivery.mode: nonejobs still avoid implicit last-target resolution.AI Assistance
AI-assisted refresh, simplification, validation, and PR-body update.