fix(cron): preserve isolated success on delivery failure by yu-xin-c · Pull Request #94116 · openclaw/openclaw

yu-xin-c · 2026-06-17T11:32:58Z

Summary

Preserve successful isolated cron agent runs when the post-run delivery phase returns an error.
Keep delivery failure metadata (error, delivered, deliveryAttempted, and diagnostics) so cron state can record not-delivered separately from agent execution status.
Add a regression test for a successful final report followed by a delivery dispatcher failure.

Root Cause

finalizeCronRun propagated a dispatchCronDelivery result with status: "error" directly when the agent payload itself was non-fatal. That collapsed delivery/finalization failures into the isolated agent execution result, so the outer scheduled run could report status=error even though the isolated session had ended successfully.

Validation

node scripts/run-vitest.mjs src/cron/isolated-agent/run.message-tool-policy.test.ts src/cron/isolated-agent/run.meta-error-status.test.ts src/cron/isolated-agent/delivery-dispatch.double-announce.test.ts src/cron/service.runs-one-shot-main-job-disables-it.test.ts src/cron/service.persists-delivered-status.test.ts
node scripts/run-tsgo.mjs -p tsconfig.core.json --incremental false
pnpm exec oxfmt --check --threads=1 src/cron/isolated-agent/run.ts src/cron/isolated-agent/run.message-tool-policy.test.ts
git diff --check

Real behavior proof

Behavior addressed: Scheduled isolated agentTurn jobs could produce a successful isolated session/final assistant message, but a post-run delivery error caused the outer cron run to persist status=error, making the success look like an agent failure.
Real environment tested: Local macOS OpenClaw source checkout at head 291dc8e6896c8cced2df1b7e5f5b5da0e100904d, with dependencies hydrated by pnpm install --frozen-lockfile and commands run from the repository root.
Exact steps or command run after this patch: Ran the focused cron runner/service regression command, core typecheck, repo formatter check, and whitespace diff check listed in Validation after applying this patch.

Evidence after fix: Terminal capture from the local OpenClaw source checkout after the patch:

$ node scripts/run-vitest.mjs src/cron/isolated-agent/run.message-tool-policy.test.ts src/cron/isolated-agent/run.meta-error-status.test.ts src/cron/isolated-agent/delivery-dispatch.double-announce.test.ts src/cron/service.runs-one-shot-main-job-disables-it.test.ts src/cron/service.persists-delivered-status.test.ts
[test] starting test/vitest/vitest.cron.config.ts

 RUN  v4.1.8 /Users/stellarfish/.openmeta/worktrees/openclaw__openclaw/openmeta__94058-cron-isolated-outcome

 Test Files  5 passed (5)
      Tests  155 passed (155)
   Start at  19:30:54
   Duration  15.08s (transform 1.90s, setup 299ms, import 3.94s, tests 10.65s, environment 0ms)

[test] passed 1 Vitest shard in 47.11s

$ node scripts/run-tsgo.mjs -p tsconfig.core.json --incremental false
# exited 0

$ pnpm exec oxfmt --check --threads=1 src/cron/isolated-agent/run.ts src/cron/isolated-agent/run.message-tool-policy.test.ts
Checking formatting...

All matched files use the correct format.
Finished in 12ms on 2 files using 1 threads.

$ git diff --check
# exited 0

Observed result after fix: The new regression path verifies runCronIsolatedAgentTurn keeps the isolated run result at status: "ok" when post-run delivery fails, while preserving error: "Message failed", delivered: false, deliveryAttempted: true, and a source: "delivery" diagnostic so the outer cron service can record delivery failure separately.
What was not tested: I did not run a live scheduled gateway/cron delivery against a real chat provider; the after-fix proof is local source-checkout terminal output plus focused runner/service regression coverage.

needtocalmdown · 2026-06-17T14:17:49Z

Thanks for the explanation.

I’m only an end user and don’t understand the code details, but your conclusion makes sense to me: the isolated agent run succeeded, while the later delivery/finalization path failed.

This also fits my situation at the time. I had some unstable channel delivery issues due to my network/proxy configuration.

One interesting detail: the issue was mostly drafted by my own OpenClaw agent. It also failed to understand the problem clearly and partly misled me into thinking this was an agent-run failure.

clawsweeper · 2026-06-18T13:19:19Z

Codex review: needs real behavior proof before merge. Reviewed June 24, 2026, 7:42 AM ET / 11:42 UTC.

Summary
The PR changes isolated cron finalization so a nonfatal isolated agent run keeps status: "ok" when post-run delivery returns an error, carries delivery metadata/diagnostics, and adds a regression test.

PR surface: Source +20, Tests +70. Total +90 across 2 files.

Reproducibility: yes. at source level: current main can return a delivery-dispatch status: "error" after successful isolated payload resolution, and scheduler persistence consumes that returned status. I did not live-run the original WSL2/systemd/provider scheduled scenario in this read-only review.

Review metrics: 1 noteworthy metric.

Cron Outcome Branch: 1 finalizer branch reclassified. The branch changes persisted cron status, delivery diagnostics, and retry/backoff behavior for an existing isolated cron delivery-failure path.

Root-cause cluster
Relationship: fixed_by_candidate
Canonical: #94058
Summary: This PR is a candidate fix for the narrow isolated cron success/error mismatch; newer and broader related PRs overlap, but none is a merged or fully review-clean replacement for closing this branch.

Members:

canonical: [Bug]: Scheduled isolated agentTurn outer run records error although session ended success #94058 - The PR body uses closing syntax for this issue and changes the isolated cron finalizer path reported there.
same_root_cause: fix(cron): keep isolated run status ok when delivery fails #95419 - This newer open PR targets the same finalizer behavior with stronger proof, but it still has an open needs-changes review finding and is not yet a safe superseding close target.
partial_overlap: [Feature]: Cron should distinguish execution success from delivery failure and show warning instead of error #49190 - It tracks the broader first-class warning-status product contract for execution success plus delivery failure.
partial_overlap: fix: record cron delivery failures as warnings #80139 - It implements the broader warning-status contract and is open/conflicting rather than a merged replacement for this narrow branch.
adjacent_distinct: [Bug]: Cron isolated agentTurn skips delivery before dispatch when a recovered early tool error is classified as fatal #94846 - It reports a different pre-dispatch recovered-tool-error delivery skip that can produce similar outer error/not-delivered symptoms.

Proposal only: this assessment does not dispatch repair, suppress jobs, mutate sibling items, close, or merge anything.

Merge readiness
Overall: 🧂 unranked krab
Proof: 🦪 silver shellfish
Patch quality: 🧂 unranked krab
Result: blocked until real behavior proof from a real setup is added.

Overall follows the weaker of proof and patch quality, so missing proof can cap an otherwise strong patch.

Rank-up moves:

Preserve errorKind: "delivery-target" refusals as primary cron errors.
[P1] Add redacted real cron/gateway, CLI, or SQLite run-log proof showing after-fix status and delivery fields.
Refresh the draft/conflicting branch against current main.

Proof guidance:

[P1] Needs real behavior proof before merge: The PR body shows focused tests, typecheck, formatting, and diff-check terminal output, but no redacted real cron/gateway, CLI, or SQLite run-log proof of the changed behavior; private details should be redacted before posting proof. After adding proof, update the PR body; ClawSweeper should re-review automatically. If it does not, the PR author or someone with repository write access can comment @clawsweeper re-review.

Risk before merge

[P1] The branch would convert deliberate unresolved delivery-target refusals from cron errors into successful execution plus not-delivered metadata, weakening an existing guard for invalid or refused delivery targets.
[P1] The PR intentionally changes persisted cron status and backoff semantics for successful execution plus delivery failure, so maintainers need to accept the ok plus not-delivered model or choose the broader warning-status contract.
[P1] The supplied proof is only focused test/typecheck/format terminal output, and the branch is currently draft/conflicting against main.

Maintainer options:

Preserve target refusals before merge (recommended)
Update the finalizer so errorKind: "delivery-target" remains a primary cron error while true post-send delivery failures can be decoupled and proven in a real run-log path.
Accept the status contract deliberately
Maintainers may intentionally accept the change from status=error to status=ok for delivery-send failures, but should do so with clear upgrade semantics and delivery-error visibility.
Defer to the cleaner candidate
Pause or close this draft branch if maintainers prefer consolidating the fix through fix(cron): keep isolated run status ok when delivery fails #95419 or the broader warning-status work once that path is review-clean.

Next step before merge

[P1] Human/contributor action is needed because the PR is draft/conflicting, lacks real behavior proof, and changes cron status semantics; do not queue an automated repair marker while proof remains contributor-owned.

Security
Cleared: The diff only changes cron TypeScript runtime logic and a focused test; it adds no dependency, workflow, secret, install, package, or supply-chain surface.

Review findings

[P1] Preserve delivery-target refusals as errors — src/cron/isolated-agent/run.ts:1306

Review details

Best possible solution:

Refresh or replace the branch with a narrow finalizer fix that preserves delivery-target errors, proves the real persisted cron/run-log behavior, and makes the ok plus not-delivered status decision explicit.

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

Yes at source level: current main can return a delivery-dispatch status: "error" after successful isolated payload resolution, and scheduler persistence consumes that returned status. I did not live-run the original WSL2/systemd/provider scheduled scenario in this read-only review.

Is this the best way to solve the issue?

No, not as submitted. finalizeCronRun is the right narrow boundary, but the patch must preserve delivery-target refusals as errors and include real persisted cron/run-log proof before it is a safe fix.

Full review comments:

[P1] Preserve delivery-target refusals as errors — src/cron/isolated-agent/run.ts:1306
This branch reclassifies every non-aborted delivery-dispatch status: "error" after a nonfatal agent payload as ok. dispatchCronDelivery returns unresolved required delivery targets with errorKind: "delivery-target", so invalid or refused targets would stop counting as cron errors/backoff instead of preserving the current guard behavior.
Confidence: 0.9

Overall correctness: patch is incorrect
Overall confidence: 0.88

AGENTS.md: found and applied where relevant.

Codex review notes: model internal, reasoning high; reviewed against 1069c60e1e25.

Label changes

Label justifications:

P2: The PR targets a bounded cron status/delivery correctness bug with medium operator impact and limited runtime scope.
merge-risk: 🚨 compatibility: Merging changes persisted cron status and backoff semantics for existing delivery-failure cases that previously appeared as error.
merge-risk: 🚨 message-delivery: The diff changes how failed scheduled completion delivery is surfaced and risks hiding invalid delivery-target refusals as successful runs.
merge-risk: 🚨 session-state: The diff changes persisted cron run status, delivery metadata, diagnostics, and scheduler counters for isolated cron runs.
rating: 🧂 unranked krab: Overall readiness is 🧂 unranked krab; proof is 🦪 silver shellfish and patch quality is 🧂 unranked krab.
status: 📣 needs proof: The PR needs real behavior proof before ClawSweeper can clear the contributor ask. Needs real behavior proof before merge: The PR body shows focused tests, typecheck, formatting, and diff-check terminal output, but no redacted real cron/gateway, CLI, or SQLite run-log proof of the changed behavior; private details should be redacted before posting proof. After adding proof, update the PR body; ClawSweeper should re-review automatically. If it does not, the PR author or someone with repository write access can comment @clawsweeper re-review.

Evidence reviewed

PR surface:

Source +20, Tests +70. Total +90 across 2 files.

View PR surface stats

Area	Files	Added	Removed	Net
Source	1	25	5	+20
Tests	1	70	0	+70
Docs	0	0	0	0
Config	0	0	0	0
Generated	0	0	0	0
Other	0	0	0	0
Total	2	95	5	+90

What I checked:

PR head broadly reclassifies delivery errors: The PR head returns resolveRunOutcome for any non-aborted deliveryResult.result.status === "error" after a nonfatal isolated payload, without excluding errorKind: "delivery-target". (src/cron/isolated-agent/run.ts:1306, 291dc8e6896c)
Delivery-target refusals are deliberate errors: Current main's delivery dispatcher creates unresolved required delivery targets as status: "error" with errorKind: "delivery-target", which the PR's broad condition would convert to ok. (src/cron/isolated-agent/delivery-dispatch.ts:934, 1069c60e1e25)
Cron status affects persisted state and backoff: applyJobResult writes lastRunStatus/lastStatus from the returned status, derives delivery state from result.error, and increments consecutive error/backoff behavior only for status === "error". (src/cron/service/timer.ts:681, 1069c60e1e25)
Current main still has the source-level collapse path: Current main returns the delivery-dispatch result verbatim when the isolated payload is nonfatal, so the reported status-collapse problem is still relevant and the PR is not obsolete. (src/cron/isolated-agent/run.ts:1322, 1069c60e1e25)
Live PR state is not merge-ready: Live GitHub data reports this PR open, draft, and conflicting, with labels including status: 📣 needs proof. (291dc8e6896c)
Overlapping cleaner candidate is not a safe close target yet: GitHub search found fix(cron): keep isolated run status ok when delivery fails #95419 as an open clean same-root candidate with stronger proof, but its current ClawSweeper review still has a needs-changes finding, so this PR should not be closed as safely superseded. (cbe864868b76)

Likely related people:

vincentkoc: Current shallow-checkout blame for the central finalizer, delivery-target construction, and scheduler persistence lines points to a recent main commit by Vincent Koc; confidence is limited by the grafted checkout history. (role: recent area contributor; confidence: medium; commits: 54c0048d6c9c, ce0142f04e; files: src/cron/isolated-agent/run.ts, src/cron/isolated-agent/delivery-dispatch.ts, src/cron/service/timer.ts)
steipete: History shows Peter Steinberger authored the delivery-dispatch extraction that created the current isolated cron delivery/finalization boundary involved here. (role: feature-history owner; confidence: medium; commits: 7a40d99b1d4d; files: src/cron/isolated-agent/delivery-dispatch.ts, src/cron/isolated-agent/run.ts)
YuzuruS: The prior merged announce-delivery status fix separated delivery failure from successful cron execution in adjacent dispatcher logic. (role: adjacent prior fix author; confidence: medium; commits: 6513c42d2d63; files: src/cron/isolated-agent/delivery-dispatch.ts)
Alix-007: Recent merged cron retry/backoff work touched scheduler status behavior, and this person also opened the newer same-root candidate PR. (role: recent adjacent scheduler contributor; confidence: medium; commits: 16fba65cb6ad, cbe864868b76; files: src/cron/service/timer.ts, src/cron/isolated-agent/run.ts)

What the crustacean ranks mean

🦀 challenger crab: rare, exceptional readiness with strong proof, clean implementation, and convincing validation.
🦞 diamond lobster: very strong readiness with only minor maintainer review expected.
🐚 platinum hermit: good normal PR, likely mergeable with ordinary maintainer review.
🦐 gold shrimp: useful signal, but proof or patch confidence is still limited.
🦪 silver shellfish: thin signal; proof, validation, or implementation needs work.
🧂 unranked krab: not merge-ready because proof is missing/unusable or there are serious correctness or safety concerns.
🌊 off-meta tidepool: rating does not apply to this item.

Shiny media proof means a screenshot, video, or linked artifact directly shows the changed behavior. Runtime, network, CSP, and security claims still need visible diagnostics.

How this review workflow works

ClawSweeper keeps one durable marker-backed review comment per issue or PR.
Re-runs edit this comment so the latest verdict, findings, and automation markers stay together instead of adding duplicate bot comments.
A fresh review can be triggered by eligible @clawsweeper re-review comments, exact-item GitHub events, scheduled/background review runs, or manual workflow dispatch.
PR/issue authors and users with repository write access can comment @clawsweeper re-review or @clawsweeper re-run on an open PR or issue to request a fresh review only.
Maintainers can also comment @clawsweeper review to request a fresh review only.
Fresh-review commands do not start repair, autofix, rebase, CI repair, or automerge.
Maintainer-only repair and merge flows require explicit commands such as @clawsweeper autofix, @clawsweeper automerge, @clawsweeper fix ci, or @clawsweeper address review.
Maintainers can comment @clawsweeper explain to ask for more context, or @clawsweeper stop to stop active automation.

needtocalmdown · 2026-06-19T07:17:09Z

Hi @yu-xin-c , I ran into a similar issue again and captured more detailed evidence, including a reduced scheduled repro.

I opened a new issue here: #94846.

It is related to this one, but the key difference is that delivery was not triggered at all. The isolated agentTurn completed successfully and produced final output, but cron finalization appears to return before dispatchCronDelivery(...) is reached.

Could you take a look if you are interested?

clawsweeper · 2026-07-03T10:54:26Z

@yu-xin-c thanks for the PR. ClawSweeper is still waiting on real behavior proof before this can move forward.

Useful proof can be a screenshot, short video, terminal output, copied live output, linked artifact, or redacted logs that show the changed behavior after the fix. Please redact private tokens, phone numbers, private endpoints, customer data, and anything else sensitive.

Once proof is added to the PR body or a comment, ClawSweeper or a maintainer can re-check it.

steipete · 2026-07-12T09:21:25Z

Superseded by the landed complete fix #105215 / 9bdb2a5ac08deafb6c1feb747530a31b395392a7; #94058 is closed as fixed.

The final implementation keeps successful execution at status=ok while preserving delivery failure separately, and covers scheduled, manual, best-effort, startup catch-up, removed-job, event, and run-log paths with 261 focused tests plus exact-head CI.

Thank you for the earlier fix and regression coverage, @yu-xin-c.

fix(cron): preserve isolated success on delivery failure

291dc8e

clawsweeper Bot mentioned this pull request Jun 18, 2026

[Bug]: Scheduled isolated agentTurn outer run records error although session ended success #94058

Closed

clawsweeper Bot added rating: 🧂 unranked krab Not merge-ready due to missing proof or serious correctness/safety concerns. and removed rating: 🦪 silver shellfish Thin PR readiness signal; proof, validation, or implementation needs work. labels Jun 20, 2026

This was referenced Jun 20, 2026

fix: record cron delivery failures as warnings #80139

Open

fix(cron): keep isolated run status ok when delivery fails #95419

Closed

steipete mentioned this pull request Jul 12, 2026

fix(cron): preserve successful run status after delivery failure #105215

Merged

steipete closed this Jul 12, 2026

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

Uh oh!

fix(cron): preserve isolated success on delivery failure#94116

fix(cron): preserve isolated success on delivery failure#94116
yu-xin-c wants to merge 1 commit into
openclaw:mainfrom
yu-xin-c:openmeta/94058-cron-isolated-outcome

yu-xin-c commented Jun 17, 2026 •

edited

Loading

Uh oh!

needtocalmdown commented Jun 17, 2026

Uh oh!

clawsweeper Bot commented Jun 18, 2026 •

edited

Loading

Uh oh!

needtocalmdown commented Jun 19, 2026

Uh oh!

clawsweeper Bot commented Jul 3, 2026

Uh oh!

steipete commented Jul 12, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

Uh oh!

Uh oh!

Conversation

yu-xin-c commented Jun 17, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Summary

Root Cause

Validation

Real behavior proof

Uh oh!

needtocalmdown commented Jun 17, 2026

Uh oh!

clawsweeper Bot commented Jun 18, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

needtocalmdown commented Jun 19, 2026

Uh oh!

clawsweeper Bot commented Jul 3, 2026

Uh oh!

steipete commented Jul 12, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

yu-xin-c commented Jun 17, 2026 •

edited

Loading

clawsweeper Bot commented Jun 18, 2026 •

edited

Loading