fix(cron): route failure alerts to configured threads#89869
Conversation
|
Codex review: needs real behavior proof before merge. Reviewed June 3, 2026, 10:36 AM ET / 14:36 UTC. Summary PR surface: Source +96, Tests +412, Docs +15, Generated +2. Total +525 across 30 files. Reproducibility: yes. from source inspection: current main's failure-alert type, resolver, gateway target, and persistence path lack a thread/topic field while primary cron delivery has one. I did not run a live Telegram cron, so this is source-reproducible rather than live reproduced. Review metrics: 1 noteworthy metric.
Merge readiness Overall follows the weaker of proof and patch quality, so missing proof can cap an otherwise strong patch. Rank-up moves:
Proof guidance:
Mantis proof suggestion Risk before merge
Maintainer options:
Next step before merge
Security Review detailsBest possible solution: Merge the additive cron failure-alert thread routing after live Telegram topic proof shows a failed cron alert lands in the configured forum topic. Do we have a high-confidence way to reproduce the issue? Yes from source inspection: current main's failure-alert type, resolver, gateway target, and persistence path lack a thread/topic field while primary cron delivery has one. I did not run a live Telegram cron, so this is source-reproducible rather than live reproduced. Is this the best way to solve the issue? Yes for the primary ask: mirroring the existing delivery AGENTS.md: found and applied where relevant. Codex review notes: model gpt-5.5, reasoning high; reviewed against d3ab7e92ef2d. Label changesLabel changes:
Label justifications:
Evidence reviewedPR surface: Source +96, Tests +412, Docs +15, Generated +2. Total +525 across 30 files. View PR surface stats
What I checked:
Likely related people:
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. How this review workflow works
|
|
Closing this draft to reduce my active PR queue so newer ready fixes can be submitted under the repository's active-PR limit. The branch remains available on the fork if maintainers want to revive this cron failure-alert thread work later. |
Summary
failureAlert.threadIdto cron types, config/tool schema, CLI edit, docs, and SQLite persistence/backfill.delivery.threadIdonly when the failure alert uses the primary announce target; avoid carrying a stale topic when channel or account differs.Fixes #89848.
Real behavior proof
Behavior addressed: Cron failure alerts now carry an explicit
failureAlert.threadId, or inheritdelivery.threadIdwhen they use the primary announce target.Real environment tested: Local OpenClaw source tree runtime through Node and tsx on this branch.
Exact steps or command run after this patch:
Evidence after fix:
{"explicit":79,"inherited":"79","accountMismatch":null}Observed result after fix: Explicit alerts resolved to topic 79, primary-target alerts inherited topic "79", and a different account did not inherit the primary topic.
What was not tested: Live Telegram delivery against a real forum-mode supergroup was not exercised in this branch run; outbound routing was verified through local cron and gateway paths.
Validation
node scripts/run-vitest.mjs src/cron/service.failure-alert.test.ts src/cron/delivery.failure-notify.test.ts src/gateway/server-cron-notifications.test.ts src/gateway/server-cron.test.ts src/agents/tools/cron-tool.schema.test.ts src/agents/tools/cron-tool.test.ts src/cli/cron-cli.test.ts src/state/openclaw-state-db.test.ts src/cron/store.test.ts src/cron/normalize.test.ts src/cron/service.jobs.test.ts src/config/schema.test.ts --reporter=dotpassed 7 shards, 474 tests.node scripts/run-tsgo.mjs -p test/tsconfig/tsconfig.core.test.jsonnode scripts/run-oxlint.mjs ...changed TS files...npx oxfmt --check ...changed TS files...pnpm docs:listgit diff --check