Skip to content

fix(cron): route failure alerts to configured threads#89869

Closed
sweetcornna wants to merge 1 commit into
openclaw:mainfrom
sweetcornna:fix/89848-cron-failure-alert-thread-id
Closed

fix(cron): route failure alerts to configured threads#89869
sweetcornna wants to merge 1 commit into
openclaw:mainfrom
sweetcornna:fix/89848-cron-failure-alert-thread-id

Conversation

@sweetcornna

Copy link
Copy Markdown
Contributor

Summary

  • Add failureAlert.threadId to cron types, config/tool schema, CLI edit, docs, and SQLite persistence/backfill.
  • Route cron failure alerts through threaded announce targets, including gateway adapter forwarding.
  • Inherit delivery.threadId only 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 inherit delivery.threadId when 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:

node --import tsx --input-type=module <<'EOF'
import assert from "node:assert/strict";
import { resolveFailureAlert } from "./src/cron/service/failure-alerts.ts";

const state = {
  deps: {
    cronConfig: {
      failureAlert: {
        enabled: true,
        after: 1,
        cooldownMs: 0,
      },
    },
  },
};

const baseJob = {
  id: "cron-topic-proof",
  name: "topic proof",
  failureAlert: undefined,
  delivery: {
    mode: "announce",
    channel: "telegram",
    to: "-1001234567890",
    threadId: "79",
  },
  state: {},
};

const explicit = resolveFailureAlert(state, {
  ...baseJob,
  failureAlert: {
    after: 1,
    channel: "telegram",
    to: "-1001234567890",
    threadId: 79,
  },
});
const inherited = resolveFailureAlert(state, baseJob);
const accountMismatch = resolveFailureAlert(state, {
  ...baseJob,
  delivery: {
    ...baseJob.delivery,
    accountId: "primary-telegram",
  },
  failureAlert: {
    after: 1,
    channel: "telegram",
    accountId: "alert-telegram",
  },
});

assert.equal(explicit?.threadId, 79);
assert.equal(inherited?.threadId, "79");
assert.equal(accountMismatch?.threadId, undefined);
console.log(JSON.stringify({ explicit: explicit?.threadId, inherited: inherited?.threadId, accountMismatch: accountMismatch?.threadId ?? null }));
EOF

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=dot passed 7 shards, 474 tests.
  • node scripts/run-tsgo.mjs -p test/tsconfig/tsconfig.core.test.json
  • node scripts/run-oxlint.mjs ...changed TS files...
  • npx oxfmt --check ...changed TS files...
  • pnpm docs:list
  • git diff --check

@openclaw-barnacle openclaw-barnacle Bot added docs Improvements or additions to documentation gateway Gateway runtime cli CLI command changes agents Agent runtime and tooling size: L proof: supplied External PR includes structured after-fix real behavior proof. labels Jun 3, 2026
@clawsweeper

clawsweeper Bot commented Jun 3, 2026

Copy link
Copy Markdown
Contributor

Codex review: needs real behavior proof before merge. Reviewed June 3, 2026, 10:36 AM ET / 14:36 UTC.

Summary
The PR adds failureAlert.threadId across cron config/types, agent tool schema, CLI edit, docs, SQLite persistence/backfill, cron resolution, gateway forwarding, and focused tests.

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.

  • Cron contract surfaces: 1 config field, 1 tool schema field, 1 CLI flag, 1 SQLite column added. The PR expands user-facing cron configuration and persisted state, so additive upgrade behavior and documentation need maintainer attention before merge.

Merge readiness
Overall: 🦐 gold shrimp
Proof: 🦐 gold shrimp
Patch quality: 🐚 platinum hermit
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:

  • [P1] Add redacted live Telegram forum-topic proof showing a cron failure alert delivered to the configured topic.

Proof guidance:

  • [P1] Needs stronger real behavior proof before merge: The PR includes terminal proof of local resolver behavior, but not a real Telegram forum-topic delivery proof for the user-visible routing change; please redact private IDs, phone numbers, tokens, endpoints, and other sensitive details when adding proof, and updating the PR body should trigger a fresh ClawSweeper review automatically. 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.

Mantis proof suggestion
A native Telegram Desktop recording would directly prove that a failed cron alert lands in the configured forum topic instead of the default thread. A maintainer can ask Mantis to capture proof by posting a new PR comment that starts with the OpenClaw Mantis account mention, followed by:

telegram desktop proof: verify a cron failure alert with failureAlert.threadId lands in the configured Telegram forum topic.

Risk before merge

  • [P1] The PR has local terminal proof but no live Telegram forum-topic delivery proof, even though the changed behavior is visible topic routing.
  • [P1] The PR adds a new optional config/tool/CLI/persisted field; the upgrade path looks additive, but maintainers should still review that this is the intended permanent cron failure-alert contract.

Maintainer options:

  1. Require live Telegram topic proof (recommended)
    Ask for a redacted live Telegram forum-topic run showing a cron failure alert delivered to the configured topic before merging.
  2. Accept local proof only
    Maintainers may choose to accept the local resolver/gateway proof and own the remaining live-topic uncertainty.

Next step before merge

  • [P1] Wait for live Telegram proof and maintainer review of the new cron failure-alert contract; there is no narrow ClawSweeper repair because the remaining blocker is runtime evidence, not a specific code defect.

Security
Cleared: No concrete security or supply-chain concern was found in the config, CLI, persistence, docs, and delivery-routing diff.

Review details

Best 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 threadId contract onto failure alerts, with careful inheritance only for the primary announce target, is the narrow maintainable fix. The optional sugar from the linked issue can stay out of this PR unless maintainers explicitly want that extra config API.

AGENTS.md: found and applied where relevant.

Codex review notes: model gpt-5.5, reasoning high; reviewed against d3ab7e92ef2d.

Label changes

Label changes:

  • add P2: The PR addresses misrouted cron failure alerts for topic-organized Telegram workspaces with a bounded cron/gateway surface.
  • add merge-risk: 🚨 message-delivery: The diff changes failure-alert message routing into Telegram topics, and local tests do not prove real Telegram delivery lands in the intended forum topic.
  • add rating: 🦐 gold shrimp: Overall readiness is 🦐 gold shrimp; proof is 🦐 gold shrimp and patch quality is 🐚 platinum hermit.
  • add status: 📣 needs proof: The PR needs real behavior proof before ClawSweeper can clear the contributor ask. Needs stronger real behavior proof before merge: The PR includes terminal proof of local resolver behavior, but not a real Telegram forum-topic delivery proof for the user-visible routing change; please redact private IDs, phone numbers, tokens, endpoints, and other sensitive details when adding proof, and updating the PR body should trigger a fresh ClawSweeper review automatically. 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.
  • add mantis: telegram-visible-proof: Mantis should capture Telegram visible proof. The PR changes visible Telegram topic routing for cron failure alerts, which is well suited to a short Telegram Desktop proof.

Label justifications:

  • P2: The PR addresses misrouted cron failure alerts for topic-organized Telegram workspaces with a bounded cron/gateway surface.
  • merge-risk: 🚨 message-delivery: The diff changes failure-alert message routing into Telegram topics, and local tests do not prove real Telegram delivery lands in the intended forum topic.
  • rating: 🦐 gold shrimp: Overall readiness is 🦐 gold shrimp; proof is 🦐 gold shrimp and patch quality is 🐚 platinum hermit.
  • status: 📣 needs proof: The PR needs real behavior proof before ClawSweeper can clear the contributor ask. Needs stronger real behavior proof before merge: The PR includes terminal proof of local resolver behavior, but not a real Telegram forum-topic delivery proof for the user-visible routing change; please redact private IDs, phone numbers, tokens, endpoints, and other sensitive details when adding proof, and updating the PR body should trigger a fresh ClawSweeper review automatically. 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.
  • mantis: telegram-visible-proof: Mantis should capture Telegram visible proof. The PR changes visible Telegram topic routing for cron failure alerts, which is well suited to a short Telegram Desktop proof.
Evidence reviewed

PR surface:

Source +96, Tests +412, Docs +15, Generated +2. Total +525 across 30 files.

View PR surface stats
Area Files Added Removed Net
Source 16 108 12 +96
Tests 8 423 11 +412
Docs 4 16 1 +15
Config 0 0 0 0
Generated 2 2 0 +2
Other 0 0 0 0
Total 30 549 24 +525

What I checked:

  • Repository policy read: Root AGENTS.md and scoped docs/agents/tools/gateway guides were read; config/persistence changes and Telegram topic behavior made upgrade/proof checks relevant. (AGENTS.md:1, d3ab7e92ef2d)
  • Current main omits failure-alert thread routing: Current resolveFailureAlert and emitFailureAlert carry channel/to/mode/accountId, but no thread/topic id, so the linked issue is source-reproducible on main. (src/cron/service/failure-alerts.ts:81, d3ab7e92ef2d)
  • Current gateway send target drops thread context: Current sendGatewayCronFailureAlert builds the announce target with channel/to/accountId/sessionKey only, which would drop any resolved failure-alert thread before delivery. (src/gateway/server-cron-notifications.ts:198, d3ab7e92ef2d)
  • PR branch carries threadId through cron resolver and delivery: The PR head resolves explicit or inherited threadId, forwards it to sendCronFailureAlert, and passes it into resolveDeliveryTarget. (src/cron/service/failure-alerts.ts:89, 404eee152cd5)
  • PR branch adds additive SQLite backfill and column creation: The PR head backfills failure_alert_thread_id from job JSON and adds the column through ensureAdditiveStateColumns, covering stored-row upgrade behavior at the schema layer. (src/state/openclaw-state-db.ts:313, 404eee152cd5)
  • Telegram proof policy applies: The Telegram maintainer note says behavior PRs touching topics need real Telegram proof, which this PR body explicitly says was not exercised against a live forum-mode supergroup. (.agents/maintainer-notes/telegram.md:37, d3ab7e92ef2d)

Likely related people:

  • steipete: Recent history includes cron notification routing and failure-alert helper work on the central runtime files this PR changes. (role: recent cron notification and storage contributor; confidence: high; commits: 85148f3b2099, 9d31cbbd6ad8; files: src/cron/delivery.ts, src/gateway/server-cron-notifications.ts, src/cron/service/failure-alerts.ts)
  • lanzhi-lee: Recent cron validation work touched adjacent tool/input paths that are relevant to accepting and normalizing a new failure-alert field. (role: recent cron input-validation contributor; confidence: medium; commits: 2664f5951916; files: src/agents/tools/cron-tool.ts, src/cron/delivery-target-validation.ts)
  • amittell: Recent schema work touched the cron tool schema and schema tests, which this PR extends for failureAlert.threadId. (role: recent cron-tool schema contributor; confidence: medium; commits: 945faf8e6798; files: src/agents/tools/cron-tool.ts, src/agents/tools/cron-tool.schema.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.

@clawsweeper clawsweeper Bot added rating: 🦐 gold shrimp Decent PR readiness signal, but merge confidence is limited. status: 📣 needs proof The PR needs real behavior proof before ClawSweeper can clear the contributor ask. mantis: telegram-visible-proof Mantis should capture Telegram visible proof. P2 Normal backlog priority with limited blast radius. merge-risk: 🚨 message-delivery 🚨 May drop, duplicate, misroute, suppress, or wrongly target messages. labels Jun 3, 2026

Copy link
Copy Markdown
Contributor Author

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.

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

Labels

agents Agent runtime and tooling cli CLI command changes docs Improvements or additions to documentation gateway Gateway runtime mantis: telegram-visible-proof Mantis should capture Telegram visible proof. merge-risk: 🚨 message-delivery 🚨 May drop, duplicate, misroute, suppress, or wrongly target messages. P2 Normal backlog priority with limited blast radius. proof: supplied External PR includes structured after-fix real behavior proof. rating: 🦐 gold shrimp Decent PR readiness signal, but merge confidence is limited. size: L status: 📣 needs proof The PR needs real behavior proof before ClawSweeper can clear the contributor ask.

Projects

None yet

1 participant