fix(cron): announce delivery for Telegram forum topics#49704
Conversation
- Pre-parse forum topic targets in delivery-target.ts before plugin bootstrap - Add slash format support to parseTelegramTarget (chatId/topicId) - Pass cfg through resolveSessionDeliveryTarget for plugin bootstrapping - Propagate deliveryError from dispatch to run result for better diagnostics - Update cron-jobs.md to document all supported forum topic formats Fixes: cron announce delivery silently returning not-delivered for Telegram forum topics regardless of target format used.
Greptile SummaryThis PR fixes silent Key changes:
Issue found:
Confidence Score: 3/5
|
| function extractForumTopicFromTarget(to: string): { to: string; threadId: number } | null { | ||
| const stripped = to.replace(/^telegram:group:/, "").replace(/^telegram:/, ""); | ||
|
|
||
| // chatId:topic:topicId | ||
| const topicMatch = /^(-?\d+):topic:(\d+)$/.exec(stripped); | ||
| if (topicMatch) { | ||
| return { to: topicMatch[1], threadId: parseInt(topicMatch[2], 10) }; | ||
| } | ||
|
|
||
| // chatId/topicId (legacy slash) | ||
| const slashMatch = /^(-?\d+)\/(\d+)$/.exec(stripped); | ||
| if (slashMatch) { | ||
| return { to: slashMatch[1], threadId: parseInt(slashMatch[2], 10) }; | ||
| } | ||
|
|
||
| return null; | ||
| } |
There was a problem hiding this comment.
chatId:topicId shorthand not covered by pre-parser
extractForumTopicFromTarget handles :topic: (explicit keyword) and / (slash), but misses the shorthand colon format (e.g. -1001234567890:123) which is both documented as supported and matched by parseTelegramTarget's colonMatch regex.
In a fully isolated session with no gateway config (the exact environment described in the PR as the root cause), the shorthand format would still reach parseExplicitTargetWithPlugin with the Telegram plugin unloaded, silently dropping the threadId — the same bug this PR sets out to fix.
The docs change in this PR explicitly states "All formats above now work correctly for cron announce delivery," which includes -1001234567890:123. Adding a third branch to the pre-parser would close the gap:
// chatId:topicId shorthand (e.g. -1001234567890:123)
const shorthandMatch = /^(-?\d+):(\d+)$/.exec(stripped);
if (shorthandMatch) {
return { to: shorthandMatch[1], threadId: parseInt(shorthandMatch[2], 10) };
}
Note: this must be placed after the topicMatch check to avoid misinterpreting chatId:topic:topicId as a shorthand.
Prompt To Fix With AI
This is a comment left during a code review.
Path: src/cron/isolated-agent/delivery-target.ts
Line: 14-30
Comment:
**`chatId:topicId` shorthand not covered by pre-parser**
`extractForumTopicFromTarget` handles `:topic:` (explicit keyword) and `/` (slash), but misses the shorthand colon format (e.g. `-1001234567890:123`) which is both documented as supported and matched by `parseTelegramTarget`'s `colonMatch` regex.
In a fully isolated session with no gateway config (the exact environment described in the PR as the root cause), the shorthand format would still reach `parseExplicitTargetWithPlugin` with the Telegram plugin unloaded, silently dropping the `threadId` — the same bug this PR sets out to fix.
The docs change in this PR explicitly states "All formats above now work correctly for cron `announce` delivery," which includes `-1001234567890:123`. Adding a third branch to the pre-parser would close the gap:
```
// chatId:topicId shorthand (e.g. -1001234567890:123)
const shorthandMatch = /^(-?\d+):(\d+)$/.exec(stripped);
if (shorthandMatch) {
return { to: shorthandMatch[1], threadId: parseInt(shorthandMatch[2], 10) };
}
```
Note: this must be placed **after** the `topicMatch` check to avoid misinterpreting `chatId:topic:topicId` as a shorthand.
How can I resolve this? If you propose a fix, please make it concise.There was a problem hiding this comment.
💡 Codex Review
openclaw/src/cron/isolated-agent/delivery-dispatch.ts
Lines 561 to 562 in f9ea4e4
When resolvedDelivery.threadId is present, this now forces deliverViaDirect() immediately and skips finalizeTextDelivery(). That helper is the only place we wait for descendant/subagent completion and replace interim parent text, so any text-only cron run that fans out to a subagent and delivers into a Telegram forum topic will now post the parent's placeholder reply instead of the final descendant summary.
ℹ️ About Codex in GitHub
Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
| // chatId:topic:topicId | ||
| const topicMatch = /^(-?\d+):topic:(\d+)$/.exec(stripped); | ||
| if (topicMatch) { | ||
| return { to: topicMatch[1], threadId: parseInt(topicMatch[2], 10) }; |
There was a problem hiding this comment.
Parse bare
chatId:topicId targets before plugin bootstrap
This pre-parser still only recognizes chatId:topic:topicId and chatId/topicId. In the same isolated-cron case where the Telegram plugin cannot be bootstrapped, targets like -1001234567890:123 or telegram:group:-1001234567890:123 still fall through without a threadId, so delivery is attempted against the literal chat id -100…:123 and forum-topic announce remains broken for the documented shorthand format.
Useful? React with 👍 / 👎.
|
Codex review: needs real behavior proof before merge. Latest ClawSweeper review: 2026-05-22 10:02 UTC / May 22, 2026, 6:02 AM ET. Workflow note: Future ClawSweeper reviews update this same comment in place. How this review workflow works
Summary Reproducibility: yes. at source level: the PR head omits the documented PR rating Rank-up moves:
What the crustacean ranks mean
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. Real behavior proof Mantis proof suggestion Risk before merge
Maintainer options:
Next step before merge Security Review findings
Review detailsBest possible solution: Rebase onto current main, keep only the remaining accepted target-grammar and diagnostic fixes, align docs with supported syntax, and require redacted live Telegram cron topic proof before merge. Do we have a high-confidence way to reproduce the issue? Yes, at source level: the PR head omits the documented Is this the best way to solve the issue? No, not as written. The useful pieces need a current-main rebase, complete or narrowed target grammar, and real Telegram proof before this is the maintainable fix. Label justifications:
Full review comments:
Overall correctness: patch is incorrect Acceptance criteria:
What I checked:
Likely related people:
Codex review notes: model gpt-5.5, reasoning high; reviewed against 6bd430ee3517. |
|
ClawSweeper PR egg 🎁 Pass real behavior proof to wake the egg and unlock a hatchable treat. Where did the egg go?
|
|
Thanks @etticat. I replaced this with #85701 so the fix lands on current The replacement keeps the useful behavior from this PR, including Telegram legacy slash topic targets, adds focused regression coverage, updates docs/changelog, and has local focused tests plus Testbox changed-gate proof. Closing this stale/conflicted PR in favor of #85701. |
|
Replaced by #85814, which moves this work onto the modern channel target resolver/session-route APIs and keeps legacy parser compatibility out of core runtime paths. Thanks @etticat for the original report/fix direction here. The replacement keeps the Telegram forum-topic behavior covered while avoiding the old parser surface. |
Problem
Cron jobs with
delivery.mode = "announce"silently fail to deliver to Telegram forum topics. ThedeliveryStatusshows"not-delivered"with no surfaced error, even with valid targets liketelegram:-GROUP/TOPICortelegram:group:-GROUP:topic:TOPIC.Root cause:
resolveDeliveryTargetindelivery-target.tsrelies onparseExplicitTargetWithPluginto extract the threadId from forum topic target strings. But this function callsresolveOutboundChannelPlugin, which requires the Telegram plugin to be bootstrapped — and bootstrap only happens when the gateway config includes a Telegram account. In isolated cron sessions (no full gateway config), the plugin is never loaded, soparseExplicitTargetis never called, and the threadId is silently dropped.Additionally,
parseTelegramTargetdid not handle the legacy slash format (chatId/topicId), causing it to pass the full string as chatId.Fix
delivery-target.tsusing a lightweight regex parser (extractForumTopicFromTarget) that doesn't depend on plugin loading. This runs beforeresolveSessionDeliveryTargetand injects the extracted threadId asexplicitThreadId.parseTelegramTargetso-GROUP/TOPICis parsed correctly everywhere.cfgthroughresolveSessionDeliveryTarget→parseExplicitTargetWithPluginas a fallback for environments where plugin bootstrap is possible.deliveryErrorfrom dispatch through to the run result so failures surface inlastDeliveryErrorinstead of silentnot-delivered.Files changed
src/cron/isolated-agent/delivery-target.ts— core fixextensions/telegram/src/targets.ts— slash format supportsrc/infra/outbound/targets.ts— cfg passthroughsrc/cron/isolated-agent/delivery-dispatch.ts+run.ts— deliveryError propagationdocs/automation/cron-jobs.md— document all supported formatsTesting
All new tests pass. No regressions in existing test suite.