Skip to content

fix(cron): mirror direct cron deliveries into destination sessions#74913

Closed
yfge wants to merge 3 commits into
openclaw:mainfrom
yfge:fix/issue-74743
Closed

fix(cron): mirror direct cron deliveries into destination sessions#74913
yfge wants to merge 3 commits into
openclaw:mainfrom
yfge:fix/issue-74743

Conversation

@yfge

@yfge yfge commented Apr 30, 2026

Copy link
Copy Markdown
Contributor

Summary

Background

Changes

  • Resolve the cron delivery session key before direct outbound delivery instead of always reusing the agent-store session key.
  • Mirror direct cron delivery text back into that destination session transcript with the direct-delivery idempotency key.
  • Add a regression test covering session:<id> cron delivery so the outbound context and mirror both point at the destination session.

Validation

  • pnpm exec oxfmt --check --threads=1 src/cron/isolated-agent/delivery-dispatch.ts src/cron/isolated-agent/delivery-dispatch.double-announce.test.ts
  • pnpm test src/cron/isolated-agent/delivery-dispatch.double-announce.test.ts

Impact

  • Cron jobs pinned to --session session:<id> now leave the delivered assistant reply in the same persistent session transcript that users inspect later.
  • This keeps follow-up turns grounded in the actual last sent cron message instead of stale pre-cron transcript state.

@clawsweeper

clawsweeper Bot commented Apr 30, 2026

Copy link
Copy Markdown
Contributor

Thanks for the context here. I did a careful shell check against current main, and this is already implemented.

Current main already implements the useful change through the merged cron destination-session mirror work, with a broader and safer implementation than this branch. This PR branch still lacks its own real behavior proof and is now conflicted/superseded by the landed current-main path.

So I’m closing this as already implemented rather than keeping a duplicate issue open.

Review details

Best possible solution:

Keep the current-main implementation from the merged replacement PR and close this narrower conflicted branch rather than trying to land it separately.

Do we have a high-confidence way to reproduce the issue?

Yes. Source inspection and current regression tests show the reproduction path: custom session:<id> cron delivery must write the delivered assistant message into that same persistent session transcript after successful direct delivery.

Is this the best way to solve the issue?

Yes for current main, not for this branch. The merged replacement is the better solution because it resolves destination sessions, bootstraps outbound session metadata, keeps mirror failure best-effort, and preserves text in mixed media cases.

Security review:

Security review cleared: The PR diff only touches cron delivery code, focused tests, and changelog text; I found no dependency, workflow, secret, package, release, or third-party execution change.

What I checked:

Likely related people:

  • cavit99: Authored the merged replacement cron destination-session mirror work that now covers this PR's central behavior on main. (role: implemented current-main replacement; confidence: high; commits: 7de413499f1b, f3403708a3af; files: src/cron/isolated-agent/delivery-dispatch.ts, src/cron/isolated-agent/delivery-dispatch.double-announce.test.ts, src/infra/outbound/outbound-session.ts)
  • Peter Steinberger: Authored the follow-up preserving isolated delivery awareness policy and the changelog commit associated with the merged replacement. (role: recent area contributor; confidence: high; commits: c3a211993cd5, 48b4e5b3614f, 741005001b98; files: src/cron/isolated-agent/delivery-dispatch.ts, src/cron/isolated-agent/delivery-dispatch.double-announce.test.ts, CHANGELOG.md)
  • kkhomej33-netizen: Introduced session:<id> and current-session cron targeting, which defines the persistent-session behavior this fix preserves. (role: introduced custom cron session behavior; confidence: high; commits: e7d9648fba27; files: docs/automation/cron-jobs.md, src/gateway/server-cron.ts, src/cron/normalize.ts)
  • Andy: Worked on delivery mirror dedupe in the transcript append path used by the current implementation. (role: adjacent transcript mirror contributor; confidence: medium; commits: e95efa437381; files: src/config/sessions/transcript.ts)

Codex review notes: model gpt-5.5, reasoning high; reviewed against a6aa48350b7e; fix evidence: commit 7de413499f1b, main fix timestamp 2026-05-15T17:49:30Z.

@yfge

This comment was marked as spam.

@yfge

This comment was marked as spam.

@huangrichao2020

Copy link
Copy Markdown

This looks like the right small surface to land first. Mirroring the delivered text, not the whole isolated run transcript, is the important part: it keeps the worker context boundary intact while making the user-visible result part of the destination session.

One thing I'd keep in the tests if this evolves: assert the follow-up session sees the exact fallback/direct delivery text that the user saw, rather than the worker's scratch output. That avoids a class of "it is in memory, but not the same message the user received" bugs.

This also gives #74286 a clean decomposition: parent rewrite grounding can be fixed separately, while this PR handles the persistent destination transcript side.

@yfge

This comment was marked as spam.

@yfge

This comment was marked as spam.

@yfge

This comment was marked as spam.

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

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[Bug]: Custom session cron jobs do not persist in destination session.

2 participants