Skip to content

cron: honor delivery.threadId for Telegram announce delivery#505

Open
BingqingLyu wants to merge 5 commits into
mainfrom
fork-pr-43808-codex-cron-telegram-threadid
Open

cron: honor delivery.threadId for Telegram announce delivery#505
BingqingLyu wants to merge 5 commits into
mainfrom
fork-pr-43808-codex-cron-telegram-threadid

Conversation

@BingqingLyu

@BingqingLyu BingqingLyu commented Apr 27, 2026

Copy link
Copy Markdown
Owner

Summary

  • Problem: isolated cron announce delivery accepted delivery.threadId in job config, but the cron delivery plan/target plumbing never carried that field into outbound delivery.
  • Why it matters: Telegram forum-topic cron jobs fell back to General instead of the configured topic unless users encoded :topic: into delivery.to or routed through the message tool as a workaround.
  • What changed: added delivery.threadId to the cron delivery contract, normalized/preserved it on create/update, and threaded it into isolated announce target resolution.
  • What did NOT change (scope boundary): webhook delivery, failure-destination routing, and legacy payload delivery hints were left unchanged.

Change Type (select all)

  • Bug fix
  • Feature
  • Refactor
  • Docs
  • Security hardening
  • Chore/infra

Scope (select all touched areas)

  • Gateway / orchestration
  • Skills / tool execution
  • Auth / tokens
  • Memory / storage
  • Integrations
  • API / contracts
  • UI / DX
  • CI/CD / infra

Linked Issue/PR

User-visible / Behavior Changes

  • Isolated cron announce jobs now honor delivery.threadId for threaded channels such as Telegram forum topics.
  • delivery.threadId is trimmed on create/update and preserved when other delivery fields are edited.

Security Impact (required)

  • New permissions/capabilities? (Yes/No) No
  • Secrets/tokens handling changed? (Yes/No) No
  • New/changed network calls? (Yes/No) No
  • Command/tool execution surface changed? (Yes/No) No
  • Data access scope changed? (Yes/No) No
  • If any Yes, explain risk + mitigation:

Repro + Verification

Environment

  • OS: macOS
  • Runtime/container: Node 25.2.1, Bun 1.3.5
  • Model/provider: N/A
  • Integration/channel (if any): Telegram
  • Relevant config (redacted):
"delivery": {
  "mode": "announce",
  "channel": "telegram",
  "to": "-1003700845925",
  "threadId": "15",
  "bestEffort": true
}

Steps

  1. Create or edit an isolated cron job with announce delivery targeting a Telegram forum chat and set delivery.threadId to a non-General topic id.
  2. Force-run the cron job.
  3. Observe where the summary message lands.

Expected

  • The summary is delivered to the configured Telegram topic/thread.

Actual

  • Before this change, cron announce delivery ignored delivery.threadId, so the message landed in the default General topic.

Evidence

Attach at least one:

  • Failing test/log before + passing after
  • Trace/log snippets
  • Screenshot/recording
  • Perf numbers (if relevant)

Post-patch normalization/merge checks:

{"mode":"announce","channel":"telegram","to":"-1003816714067","threadId":"15"}
{"mode":"announce","channel":"telegram","to":"-100123","threadId":"15"}

Human Verification (required)

What you personally verified (not just CI), and how:

  • Verified scenarios:
    • bunx vitest run src/cron/normalize.test.ts src/cron/service.jobs.test.ts
    • Manual node --import tsx checks confirming delivery.threadId is normalized on create and preserved by applyJobPatch.
  • Edge cases checked:
    • Blank delivery.threadId is stripped.
    • Updating delivery.to preserves an existing delivery.threadId.
    • Existing :topic:-based topic routing coverage remains in place, and this PR adds explicit delivery.threadId regression coverage.
  • What you did not verify:
    • In this local workspace, the heavier Vitest files that import the cron delivery runtime (src/cron/delivery.test.ts, src/cron/isolated-agent.delivery-target-thread-session.test.ts, src/cron/isolated-agent.direct-delivery-forum-topics.test.ts) hung after RUN ... without producing a failure, so I could not complete those locally.

Review Conversations

  • I replied to or resolved every bot review conversation I addressed in this PR.
  • I left unresolved only the conversations that still need reviewer or maintainer judgment.

If a bot review conversation is addressed by this PR, resolve that conversation yourself. Do not leave bot review conversation cleanup for maintainers.

Compatibility / Migration

  • Backward compatible? (Yes/No) Yes
  • Config/env changes? (Yes/No) No
  • Migration needed? (Yes/No) No
  • If yes, exact upgrade steps:

Failure Recovery (if this breaks)

  • How to disable/revert this change quickly:
    • Revert commit 28de381c8, or omit delivery.threadId and keep using delivery.to: "<chatId>:topic:<topicId>" / the message-tool workaround.
  • Files/config to restore:
    • src/cron/types.ts
    • src/cron/delivery.ts
    • src/cron/isolated-agent/delivery-target.ts
    • src/cron/isolated-agent/run.ts
    • src/cron/normalize.ts
    • src/cron/service/jobs.ts
  • Known bad symptoms reviewers should watch for:
    • Cron announce delivery landing in the wrong Telegram topic.
    • Job edits unexpectedly clearing an existing delivery.threadId.

Risks and Mitigations

  • Risk:
    • Threaded delivery behavior could change for channels that start consuming delivery.threadId from cron config.
  • Mitigation:
    • The value is only forwarded when explicitly configured, blank values are normalized away, and the patch adds create/update regression coverage around the new field.

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

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Cron delivery ignores threadId for Telegram forum topics

2 participants