fix(cron): preserve successful run status after delivery failure#105215
Merged
Conversation
A successful isolated cron agentTurn whose post-run delivery phase returns an error collapsed the execution status into the delivery error, so the outer scheduled run persisted status=error even though the isolated session ended successfully. Decouple execution status from delivery outcome in finalizeCronRun: when the agent payload is non-fatal and the run was not aborted, a delivery dispatch error now keeps status=ok and records the failure separately via delivered/deliveryAttempted plus delivery diagnostics, which drive the already-decoupled run-log deliveryStatus (not-delivered). Aborted runs and fatal agent payloads keep their error status. Fixes #94058
The #94058 fix kept isolated turn status=ok when post-run delivery fails, but the broad `status === "error"` check also caught the #91613 keyless implicit last-target refusal, which is a deliberate delivery-target guard (errorKind "delivery-target") and must stay status=error. Exclude that errorKind so genuine delivery dispatch failures still become ok while target-guard refusals remain errors.
A successful isolated agent turn keeps status=ok when post-run delivery fails (#94058), but the ok/not-delivered resolveRunOutcome branch only forwarded delivery diagnostics and dropped the delivery dispatch error. That left lastDeliveryError empty and the finished-event deliveryError field blank, so CLI/UI/API run logs could not show why delivery did not land for an otherwise successful run. Thread the delivery error on a dedicated CronRunOutcome.deliveryError field from resolveRunOutcome through executeDetachedCronJob and applyJobResult into resolveDeliveryState, so it persists as lastDeliveryError and reaches the finished event (and thus the run log) without conflating it with a run-level error (lastError stays empty on a successful run). Add a service/run-log readback test proving the delivery error survives the cron boundary into a persisted run-log entry, and extend the isolated-turn regression to assert the dedicated deliveryError field.
openclaw-barnacle
Bot
added
docs
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 83c7425ab3
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
|
|
||
| - **Gradium TTS credential egress:** reject non-HTTPS, foreign-host, and hostname-lookalike base URLs before dispatching API keys, and pin guarded transport to Gradium's documented API hostname. (#101280) Thanks @zhangguiping-xydt. | ||
| - **Gateway command SecretRefs:** preserve authoritative active-snapshot values when another command secret remains unresolved, falling back locally only for missing paths instead of emitting a per-turn `secrets.resolve` failure. (#96661) Thanks @SunnyShu0925. | ||
| - **Cron delivery status:** keep successful isolated agent turns at `status=ok` when downstream delivery fails, while preserving the send failure separately in delivery state and run logs. (#95419) Thanks @Alix-007. |
Contributor
There was a problem hiding this comment.
The root AGENTS.md Docs/Changelog policy says CHANGELOG.md is release-only and normal fixes should put release-note context in the PR body or squash/direct commit message instead. Since this commit is a regular cron fix rather than a release-generation change, adding this line can leave stale or duplicated release notes when the generated changelog is refreshed; please drop the direct changelog edit and keep the note in the commit/PR metadata.
Useful? React with 👍 / 👎.
Contributor
Author
|
Land-ready maintainer proof:
This replacement preserves @Alix-007's contributor commits and adds the cross-path fixes required by review. |
Contributor
Author
|
Merged via squash.
|
This was referenced Jul 12, 2026
github-actions Bot
pushed a commit
to Desicool/openclaw
that referenced
this pull request
Jul 13, 2026
…nclaw#105215) * fix(cron): keep isolated run status ok when delivery fails A successful isolated cron agentTurn whose post-run delivery phase returns an error collapsed the execution status into the delivery error, so the outer scheduled run persisted status=error even though the isolated session ended successfully. Decouple execution status from delivery outcome in finalizeCronRun: when the agent payload is non-fatal and the run was not aborted, a delivery dispatch error now keeps status=ok and records the failure separately via delivered/deliveryAttempted plus delivery diagnostics, which drive the already-decoupled run-log deliveryStatus (not-delivered). Aborted runs and fatal agent payloads keep their error status. Fixes openclaw#94058 * fix(cron): keep deliberate delivery-target refusals as error status The openclaw#94058 fix kept isolated turn status=ok when post-run delivery fails, but the broad `status === "error"` check also caught the openclaw#91613 keyless implicit last-target refusal, which is a deliberate delivery-target guard (errorKind "delivery-target") and must stay status=error. Exclude that errorKind so genuine delivery dispatch failures still become ok while target-guard refusals remain errors. * fix(cron): surface successful run delivery errors to run logs A successful isolated agent turn keeps status=ok when post-run delivery fails (openclaw#94058), but the ok/not-delivered resolveRunOutcome branch only forwarded delivery diagnostics and dropped the delivery dispatch error. That left lastDeliveryError empty and the finished-event deliveryError field blank, so CLI/UI/API run logs could not show why delivery did not land for an otherwise successful run. Thread the delivery error on a dedicated CronRunOutcome.deliveryError field from resolveRunOutcome through executeDetachedCronJob and applyJobResult into resolveDeliveryState, so it persists as lastDeliveryError and reaches the finished event (and thus the run log) without conflating it with a run-level error (lastError stays empty on a successful run). Add a service/run-log readback test proving the delivery error survives the cron boundary into a persisted run-log entry, and extend the isolated-turn regression to assert the dedicated deliveryError field. * fix(cron): preserve successful delivery outcomes * fix(cron): preserve startup delivery errors * fix(cron): retain best-effort delivery errors * fix(cron): preserve delivery errors on fallback paths * fix(cron): preserve delivery errors on fallback paths --------- Co-authored-by: Alix-007 <[email protected]>
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
2 participants
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.
What Problem This Solves
Replaces #95419 on current main and fixes #94058. A successful isolated cron execution was reported as
status=errorwhen its later delivery failed, conflating execution outcome with delivery outcome. Some best-effort and startup catch-up paths also dropped the delivery error entirely.Why This Change Was Made
The final implementation keeps execution and delivery as separate state dimensions. Successful execution remains
status=ok;delivered,deliveryAttempted, anddeliveryErrorcarry post-run delivery state. It reclassifies the authoritative final result in place so final output and diagnostics survive, carries best-effort failures through the dispatcher, and preserves the error through startup catch-up projection.This preserves the contributor commits from @Alix-007 and adds maintainer fixes for final-output preservation, single-copy diagnostics, best-effort delivery errors, and startup catch-up persistence.
User Impact
Cron history and events no longer claim a successful agent run failed solely because delivery failed. Operators still see the undelivered state and its specific error in run logs, including required, best-effort, scheduled, manual, and startup catch-up paths.
Evidence
tbx_01kxarzcsm09k7x7yfpdkm2rae: 261 focused cron tests passed across seven files.pnpm tsgo:coreandpnpm tsgo:test:srcpassed.run_4eb146ece579, 177 focused tests passed.Closes #94058.
Supersedes #95419 and #94116.