cron: honor delivery.threadId for Telegram announce delivery#505
Open
BingqingLyu wants to merge 5 commits into
Open
cron: honor delivery.threadId for Telegram announce delivery#505BingqingLyu wants to merge 5 commits into
BingqingLyu wants to merge 5 commits into
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
delivery.threadIdin job config, but the cron delivery plan/target plumbing never carried that field into outbound delivery.:topic:intodelivery.toor routed through the message tool as a workaround.delivery.threadIdto the cron delivery contract, normalized/preserved it on create/update, and threaded it into isolated announce target resolution.Change Type (select all)
Scope (select all touched areas)
Linked Issue/PR
User-visible / Behavior Changes
delivery.threadIdfor threaded channels such as Telegram forum topics.delivery.threadIdis trimmed on create/update and preserved when other delivery fields are edited.Security Impact (required)
Yes/No) NoYes/No) NoYes/No) NoYes/No) NoYes/No) NoYes, explain risk + mitigation:Repro + Verification
Environment
Steps
delivery.threadIdto a non-General topic id.Expected
Actual
delivery.threadId, so the message landed in the default General topic.Evidence
Attach at least one:
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:
bunx vitest run src/cron/normalize.test.ts src/cron/service.jobs.test.tsnode --import tsxchecks confirmingdelivery.threadIdis normalized on create and preserved byapplyJobPatch.delivery.threadIdis stripped.delivery.topreserves an existingdelivery.threadId.:topic:-based topic routing coverage remains in place, and this PR adds explicitdelivery.threadIdregression coverage.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 afterRUN ...without producing a failure, so I could not complete those locally.Review Conversations
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
Yes/No) YesYes/No) NoYes/No) NoFailure Recovery (if this breaks)
28de381c8, or omitdelivery.threadIdand keep usingdelivery.to: "<chatId>:topic:<topicId>"/ the message-tool workaround.src/cron/types.tssrc/cron/delivery.tssrc/cron/isolated-agent/delivery-target.tssrc/cron/isolated-agent/run.tssrc/cron/normalize.tssrc/cron/service/jobs.tsdelivery.threadId.Risks and Mitigations
delivery.threadIdfrom cron config.