Greptile Summary

This PR fixes a gap where delivery.threadId was accepted in cron job configs but never propagated into the outbound announce delivery, causing Telegram forum-topic cron jobs to always land in the General topic instead of the configured topic.

The fix is well-scoped and consistent:

• Type contract (types.ts): threadId?: string | number added to CronDelivery; CronDeliveryPatch inherits it via Partial<CronDelivery>.
• Delivery plan (delivery.ts): New normalizeThreadId helper (mirrors existing normalizeAccountId/normalizeTo pattern) extracts and trims threadId into CronDeliveryPlan.
• Target resolution (isolated-agent/delivery-target.ts, run.ts): deliveryPlan.threadId is forwarded as explicitThreadId to resolveSessionDeliveryTarget, which already sets threadIdExplicit: true for explicitly-provided values — the downstream carry-through guard then passes it through to the resolved target.
• Normalization / patching (normalize.ts, service/jobs.ts): coerceDelivery and mergeCronDelivery are extended with the same presence-check + trim pattern used for other delivery fields; blank/invalid values are stripped, and existing threadId is preserved when unrelated fields are patched.
• Tests: Unit tests cover trimming, blank-stripping, patch-preserve, and end-to-end forum-topic delivery with messageThreadId: 15 confirmed at the Telegram send layer.

One minor observation worth awareness: isSameDeliveryTarget (failure-destination deduplication in delivery.ts) does not account for threadId. If a job delivers to a specific forum topic via explicit threadId and a failure destination happens to share the same channel/to/accountId, the deduplication would consider them equivalent and suppress the failure notification — even though one goes to a specific topic and the other would default to General. This is explicitly out of scope for this PR (failure-destination routing is listed as unchanged), but reviewers may want to track it as a follow-up.

Confidence Score: 5/5

• This PR is safe to merge — the change is additive, backward-compatible, and covers the new path with solid tests.
• The fix is minimal and well-contained: a new optional field is threaded through existing normalization and target-resolution pipelines using the same patterns already present for accountId and to. No existing behavior is altered for jobs that don't set threadId. Normalization strips blank/invalid values consistently. Tests cover trimming on create, blank-stripping on patch, preservation when unrelated fields change, and an end-to-end forum-topic delivery assertion confirming the correct numeric messageThreadId reaches the Telegram send layer. The only out-of-scope gap (failure-destination deduplication ignoring threadId) is an existing limitation, not a regression introduced by this PR.
• No files require special attention. The isSameDeliveryTarget function in src/cron/delivery.ts is a known follow-up candidate but is explicitly out of scope for this PR.

_{Last reviewed commit: 28de381}

Re-review progress:

• State: Complete
• Detail: The targeted re-review finished, the durable review comment was updated, and the synced verdict was routed.
• Run: https://github.com/openclaw/clawsweeper/actions/runs/25382067112
• Updated: 2026-05-05T14:26:19.965Z

One remaining blocker appears to be infra-only.

• checks-windows (node, test, 6, 6, pnpm test) failed in Checkout before Node/pnpm/install/test steps started.
• Windows shards 1/6 through 5/6 passed.
• check, Bun unit tests, full Node tests, and compat-node22 also passed on the rebased head.
• Rebasing onto latest main fixed the earlier unrelated src/secrets/target-registry.test.ts failure.

I also re-ran the targeted tests locally on the rebased head:

• src/secrets/target-registry.test.ts
• src/cron/normalize.test.ts
• src/cron/service.jobs.test.ts
• src/cron/isolated-agent.delivery-target-thread-session.test.ts
• src/cron/isolated-agent.direct-delivery-forum-topics.test.ts

Could a maintainer rerun the failed Windows shard or the workflow?

@tyler6204 this is ready on the rebased head 68d52bd32f1bd06d35b5bb5eb66e5ff826b65114.

The only remaining red check is checks-windows (node, test, 6, 6, pnpm test), and that job failed in Checkout before install or tests started. All other test lanes are green on this head, including Linux, Bun, and Windows shards 1/6 through 5/6.

Could you rerun that failed Windows shard or the workflow when you have a moment?

Follow-up pushed in Alex-Alaniz@656eb1d.

This addresses the remaining threadId gap in two places:

• keep failure notifications when the primary announce target uses explicit delivery.threadId
• accept and return delivery.threadId through the gateway cron schema so add/update/list round-trip cleanly

Local verification:

• bunx vitest run src/cron/delivery.test.ts
• bunx vitest run src/gateway/server.cron.test.ts --testNamePattern='handles cron CRUD, normalization, and patch semantics'
• pnpm exec oxfmt --check src/cron/delivery.ts src/cron/delivery.test.ts src/gateway/protocol/schema/cron.ts src/gateway/server.cron.test.ts
• pnpm exec oxlint src/cron/delivery.ts src/cron/delivery.test.ts src/gateway/protocol/schema/cron.ts src/gateway/server.cron.test.ts

The remaining red check job is hitting the same repo-wide formatter drift that is now failing on main, not anything in the cron diff.

Evidence:

• PR check failure: ui/src/styles/chat/grouped.css, ui/src/styles/chat/layout.css, ui/src/styles/layout.css
• current main CI is failing on the same three files in check
• this branch diff does not touch any ui/src/styles/* files

I’m leaving those CSS files out of this PR to keep the scope on the cron fix.

This pull request has been automatically marked as stale due to inactivity.
Please add updates or it will be closed.

Thanks for the context here. I swept through the related work, and this is now duplicate or superseded.

Keep open: current main still has the source-level cron failure-destination dedupe gap, and this PR is a focused repair with sufficient terminal proof, but the head is now merge-conflicting and needs adaptation to the newer delivery-target routing code before merge.

Canonical path: Close this PR as superseded by #31059.

So I’m closing this here and keeping the remaining discussion on #31059.

Added exact-head real behavior proof for the suppressSessionThreadId failure-send path.

Command run locally on the PR branch head 8f07f8cdcc335683d96efc054c3ad97d582ac20d:

pnpm exec vitest run --config test/vitest/vitest.cron.config.ts src/cron/delivery.failure-notify.test.ts src/cron/isolated-agent/delivery-target.test.ts --reporter verbose

Relevant live output:

✓ |cron| src/cron/isolated-agent/delivery-target.test.ts > resolveDeliveryTarget > suppresses session Telegram topic threadId carry for threadless failure destinations
✓ |cron| src/cron/isolated-agent/delivery-target.test.ts > resolveDeliveryTarget > suppresses session Telegram topic threadId carry when the failure destination inherits the session target
✓ |cron| src/cron/delivery.failure-notify.test.ts > sendFailureNotificationAnnounce > can suppress session-derived threadIds while preserving the failure session context

Test Files  2 passed (2)
Tests  41 passed (41)

What this proves:

• resolveDeliveryTarget(..., { suppressSessionThreadId: true }) drops the session-derived Telegram topic thread for a threadless failure destination.
• The inherited-session target case is covered too, so a failure destination using the cron delivery session key does not accidentally reuse the original topic.
• sendFailureNotificationAnnounce() passes { suppressSessionThreadId: true } into delivery-target resolution while still preserving the failure session context.

No code changes in this comment; this is the missing proof requested by ClawSweeper.

Re-review progress:

• State: Complete
• Detail: The targeted re-review finished, the durable review comment was updated, and the synced verdict was routed.
• Run: https://github.com/openclaw/clawsweeper/actions/runs/25473966402
• Updated: 2026-05-07T03:18:46.963Z

ClawSweeper PR egg

✨ Hatched: ✨ glimmer Neon Branchling

Hatch command

Comment @clawsweeper hatch when this PR is hatchable.

Hatchability rules:

• Merged PRs are hatchable.
• Open PRs are hatchable when they are status: 👀 ready for maintainer look, status: 🚀 automerge armed, or labeled clawsweeper:automerge.
• Closed unmerged PRs are hatchable only when one of those hatchable labels is still present in the durable record.

Rarity: ✨ glimmer.
Trait: stacks clean commits.
Image traits: location workflow harbor; accessory release bell; palette coral, mint, and warm cream; mood curious; pose standing beside its cracked shell; shell soft velvet shell; lighting moonlit rim light; background subtle branch markers.
Share on X: post this hatch
Copy: My PR egg hatched a ✨ glimmer Neon Branchling in ClawSweeper.

ClawSweeper applied the proposed close for this PR.

• Action: closed this PR.
• Close reason: duplicate or superseded.
• Evidence: durable ClawSweeper review.