fix(cron): treat cron tool warnings as non-fatal when run recovered by LiLan0125 · Pull Request #94855 · openclaw/openclaw

LiLan0125 · 2026-06-19T07:28:03Z

Summary

When an isolated cron agentTurn encounters a recoverable early tool error, the warning payload was classified as fatal even when the agent recovered and produced final output ([Bug]: Cron isolated agentTurn skips delivery before dispatch when a recovered early tool error is classified as fatal #94846).
Fix 1: Extend hasNonTerminalToolErrorWarning to also match the cron tool warning text pattern (isCronToolWarning), not only the metadata flag.
Fix 2 (per ClawSweeper review): When a recovered tool warning is non-fatal and final assistant text exists, prefer the final text for delivery even without the channel preference flag. This prevents the warning payload from being dispatched instead of the recovered output.

Verification

node scripts/run-vitest.mjs run src/cron/isolated-agent/helpers.test.ts → 25/25 passed

Impact Assessment

Scope: src/cron/isolated-agent/helpers.ts — widen non-terminal classification + prefer final text on recovery
Fix 1: isCronToolWarning text pattern now triggers nonTerminalToolErrorWarning alongside metadata flag
Fix 2: hasRecoveredNonFatalWithFinalText gate allows shouldUseFinalAssistantVisibleText without preferFinalAssistantVisibleText
Structured/media payloads are never replaced by final text (guarded by hasStructuredDeliveryPayloads)

Real behavior proof

Behavior addressed: Recovered cron tool warnings that produced final output had their warning text dispatched instead of the final assistant report. After both fixes, non-fatal classification + final-text preference ensures recovered output is delivered.

Environment tested: Node 22.x on Linux, vitest test suite.

Steps run after the patch: Ran the cron helpers test suite.

Evidence after fix:

Observed result: All 25 tests pass. The hasNonTerminalToolErrorWarning gate now covers both metadata and text patterns, and recovered final text is preferred over warning payloads.

Not tested: Live scheduled cron job with Feishu delivery was not exercised.

Closes #94846

When an isolated cron agentTurn encounters a recoverable early tool error, the warning payload was classified as fatal even when the agent successfully recovered and produced final output. This caused delivery to be skipped and the run recorded as an error. Extend hasNonTerminalToolErrorWarning to also match the cron tool warning text pattern (isCronToolWarning), not only the metadata flag (nonTerminalToolErrorWarning). Closes openclaw#94846

clawsweeper · 2026-06-19T07:29:05Z

Codex review: needs real behavior proof before merge. Reviewed June 21, 2026, 9:40 AM ET / 13:40 UTC.

Summary
The branch changes resolveCronPayloadOutcome so recovered cron tool-warning text can be treated as non-fatal and final assistant text can replace warning delivery payloads without the channel preference flag.

PR surface: Source +11. Total +11 across 1 file.

Reproducibility: yes. at source level: current main still gates recovered cron tool-warning recovery on channel final-text preference and returns before dispatch when the warning remains fatal. I did not run a live scheduled Feishu cron delivery in this read-only review.

Review metrics: 1 noteworthy metric.

Changed resolver gates: 2 changed, 0 resolver tests added. The diff changes fatality classification and final-text delivery selection, so maintainers need focused coverage before merge.

Root-cause cluster
Relationship: fixed_by_candidate
Canonical: #94846
Summary: This PR is one candidate fix for the canonical recovered isolated-cron tool-warning pre-dispatch delivery skip; sibling PRs overlap the same issue with different resolver and proof shapes.

Members:

canonical: [Bug]: Cron isolated agentTurn skips delivery before dispatch when a recovered early tool error is classified as fatal #94846 - It is the open issue this PR closes and contains the reduced scheduled Feishu reproduction for the recovered tool-warning pre-dispatch skip.
fixed_by_candidate: fix(cron): detect recovered tool warnings without preferFinalAssistantVisibleText #94849 - This open PR also uses closing syntax for the canonical issue and removes the channel-preference guard from recovered warning classification.
fixed_by_candidate: fix(cron): allow recovered early tool errors to succeed delivery (#94846) #94862 - This open PR also uses closing syntax for the canonical issue with a broader run-level-error recovery approach.
duplicate: Bug: isolated agentTurn cron marked error when exec exits 1, even when agent recovers and produces correct output #94863 - The closed issue reports the same recovered isolated cron tool-warning classification failure for a different channel and script shape.
partial_overlap: [Bug]: Scheduled isolated agentTurn outer run records error although session ended success #94058 - It covers the broader isolated-session-success versus parent-cron-error boundary, while this PR targets the narrower recovered tool-warning pre-dispatch path.
1 more in the report.

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

Merge readiness
Overall: 🧂 unranked krab
Proof: 🧂 unranked krab
Patch quality: 🦐 gold shrimp
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:

[P1] Add redacted real behavior proof from a scheduled or direct-delivery cron run showing final assistant text delivered after a recovered tool warning.
[P1] Add resolver tests for the preference-false recovered-warning path and a mixed-error fatal control.
Coordinate with the canonical issue's sibling PRs so maintainers land one tested shape.

Proof guidance:

[P1] Needs real behavior proof before merge: The PR body supplies only unit-test proof and explicitly says no live scheduled cron job with Feishu delivery was exercised; add redacted terminal output, logs, or a recording from a real scheduled or direct-delivery cron run. 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.

Mantis proof suggestion
A real channel proof would materially show that a recovered cron run delivers final assistant text instead of the warning payload. A maintainer can ask Mantis to capture proof by posting this exact PR comment:

@openclaw-mantis slack desktop smoke: verify a scheduled isolated cron run with a recovered tool warning delivers final assistant text, not the warning.

Risk before merge

[P1] The PR changes whether recovered cron tool-warning runs dispatch and what text they deliver, so it can alter persisted run status and user-visible channel messages.
[P1] The resolver suite lacks a preference-false recovered-warning regression and a mixed-error fatal control for the exact changed gates.
[P1] Several open PRs still target the same canonical issue with different helper and run-level semantics, so maintainers need one canonical landing shape.

Maintainer options:

Require tests and real delivery proof (recommended)
Ask for focused resolver regressions plus redacted scheduled or direct-delivery cron proof showing final assistant text reaches a channel after a recovered tool warning.
Choose one canonical sibling fix
If another open candidate has the cleaner tested shape, pause this branch until that replacement is selected or merged.
Accept helper-only evidence deliberately
Maintainers can accept the narrow helper change after CI, but that leaves the reported channel delivery path unproven.

Next step before merge

[P1] Needs contributor real behavior proof and maintainer selection among overlapping linked fixes before this branch can merge.

Security
Cleared: No concrete security or supply-chain concern found; the diff is limited to TypeScript cron helper logic and adds no dependency, workflow, secret, install, or packaging surface.

Review findings

[P2] Add resolver coverage for recovered warning selection — src/cron/isolated-agent/helpers.ts:280-281

Review details

Best possible solution:

Land one canonical cron payload-outcome fix that proves preference-false recovered-warning delivery, preserves genuine fatal structured-error suppression, and supersedes the overlapping candidate PRs after merge.

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

Yes, at source level: current main still gates recovered cron tool-warning recovery on channel final-text preference and returns before dispatch when the warning remains fatal. I did not run a live scheduled Feishu cron delivery in this read-only review.

Is this the best way to solve the issue?

Not yet. The helper layer is a plausible fix location, but this branch still needs resolver regressions, real delivery proof, and maintainer selection among overlapping candidate PRs.

Full review comments:

[P2] Add resolver coverage for recovered warning selection — src/cron/isolated-agent/helpers.ts:280-281
This diff changes resolveCronPayloadOutcome fatality and payload-selection behavior, but it adds no regression for a preference-false cron tool-warning recovery or a mixed-error fatal control. Please add focused resolver coverage so this path cannot silently send the warning text or clear unrelated errors later.
Confidence: 0.84

Overall correctness: patch is incorrect
Overall confidence: 0.84

AGENTS.md: found and applied where relevant.

Codex review notes: model internal, reasoning high; reviewed against 11a2e03bd4de.

Label changes

Label justifications:

P2: The PR targets a bounded cron delivery/status bug with meaningful scheduled-report impact but no crash, data loss, or security bypass.
merge-risk: 🚨 message-delivery: The diff changes whether recovered cron tool-warning runs dispatch and which text is selected for channel delivery.
merge-risk: 🚨 session-state: The diff changes fatal-versus-recovered cron outcome classification, affecting persisted run status and diagnostics.
rating: 🧂 unranked krab: Overall readiness is 🧂 unranked krab; proof is 🧂 unranked krab and patch quality is 🦐 gold shrimp.
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 supplies only unit-test proof and explicitly says no live scheduled cron job with Feishu delivery was exercised; add redacted terminal output, logs, or a recording from a real scheduled or direct-delivery cron run. 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 +11. Total +11 across 1 file.

View PR surface stats

Area	Files	Added	Removed	Net
Source	1	13	2	+11
Tests	0	0	0	0
Docs	0	0	0	0
Config	0	0	0	0
Generated	0	0	0	0
Other	0	0	0	0
Total	1	13	2	+11

What I checked:

Repository policy read: Root AGENTS.md was read fully; no scoped AGENTS.md exists under src/cron, so root deep-review, real-proof, and merge-risk guidance applies. (AGENTS.md:1, 11a2e03bd4de)
Current main recovery gate: Current main still requires preferFinalAssistantVisibleText === true for the existing recovered tool-warning path, so preference-false channels remain in the reported fatal-classification shape. (src/cron/isolated-agent/helpers.ts:289, 11a2e03bd4de)
Current main pre-dispatch skip: Cron finalization passes finalRunResult.meta?.error into the resolver and returns before dispatchCronDelivery when hasFatalStructuredErrorPayload remains true. (src/cron/isolated-agent/run.ts:1143, 11a2e03bd4de)
PR head patch: The PR widens non-terminal classification to cron tool-warning text when all error payloads match, then allows recovered final assistant text to replace warning payloads. (src/cron/isolated-agent/helpers.ts:280, df46811d21ae)
Changed-surface test gap: The resolver test file already has preferred-final-text coverage, but this PR adds no regression for the preference-false recovered-warning path or mixed-error fatal control. (src/cron/isolated-agent.helpers.test.ts:32, 11a2e03bd4de)
Proof gap: The PR body reports only a helper-suite Vitest run and explicitly says no live scheduled cron job with Feishu delivery was exercised. (df46811d21ae)

Likely related people:

sercada: Authored the merged fatal structured-error completion-announce suppression PR whose non-dispatch boundary must remain correct for genuine fatal errors. (role: introduced related behavior; confidence: high; commits: 2e6ca9f703da, 0c7220f5da78; files: src/cron/isolated-agent/run.ts, src/cron/isolated-agent/helpers.ts, src/cron/isolated-agent/run.message-tool-policy.test.ts)
steipete: Merged the fatal suppression PR and authored adjacent cron final-output-over-tool-warning work on the same helper/finalization boundary. (role: merger and adjacent contributor; confidence: high; commits: 0c7220f5da78, c8a953af9371, 552e0bdf7616; files: src/cron/isolated-agent/helpers.ts, src/cron/isolated-agent/run.ts, src/cron/isolated-agent.helpers.test.ts)
welfo-beo: Authored the earlier cron final announce delivery work that introduced final assistant visible-text handling into this resolver surface. (role: adjacent feature contributor; confidence: medium; commits: 01bfe4f25e3f, 81c7304a18b8; files: src/cron/isolated-agent/helpers.ts, src/cron/isolated-agent.helpers.test.ts, src/cron/isolated-agent/run.ts)
hydro13: Authored merged cron announce payload selection work that changed the same resolver and tests. (role: payload selection contributor; confidence: medium; commits: 779d2ce830ff, bdd9bc93f12c; files: src/cron/isolated-agent/helpers.ts, src/cron/isolated-agent.helpers.test.ts, src/cron/isolated-agent/helpers.test.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.

…payload When a recovered isolated cron run has a non-fatal tool warning and a final assistant-visible text, prefer the final text for delivery even when the channel preference flag is not set. This prevents the warning payload from being dispatched instead of the recovered output. Refs openclaw#94846

LiLan0125 · 2026-06-19T09:03:11Z

@clawsweeper re-review

Added fix for the P1 payload-selection issue: when a recovered tool warning is non-fatal and final assistant text exists, prefer the final text for delivery even without the channel preference flag. This prevents the warning payload from being dispatched instead of the recovered output.

clawsweeper · 2026-06-19T09:03:14Z

🦞🧹
ClawSweeper re-review requested.

I asked ClawSweeper to review this item again.
Action: item re-review queued (workflow sweep.yml, event repository_dispatch).
Result: the existing ClawSweeper review comment will be edited in place when the review finishes.

…ring fatality Per ClawSweeper review: when checking isCronToolWarning on the last error text, also verify that ALL error payloads match the tool warning pattern. This prevents mixed-error scenarios where a genuinely fatal error payload precedes a tool warning from being incorrectly classified as non-fatal. Refs openclaw#94846

LiLan0125 · 2026-06-19T10:59:17Z

@clawsweeper re-review

Tightened the recovered-warning predicate per P1 feedback: now requires — all error payloads must be cron tool warnings before clearing fatality, not just the last one. This prevents mixed-error scenarios from being incorrectly classified as non-fatal.

clawsweeper · 2026-06-19T10:59:20Z

🦞🧹
ClawSweeper re-review requested.

I asked ClawSweeper to review this item again.
Action: item re-review queued (workflow sweep.yml, event repository_dispatch).
Result: the existing ClawSweeper review comment will be edited in place when the review finishes.

openclaw-barnacle Bot added size: XS proof: supplied External PR includes structured after-fix real behavior proof. labels Jun 19, 2026

openclaw-barnacle Bot added triage: needs-real-behavior-proof Candidate: external PR needs after-fix proof from a real setup. and removed proof: supplied External PR includes structured after-fix real behavior proof. labels Jun 19, 2026

LiLan0125 closed this Jul 1, 2026

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

Uh oh!

fix(cron): treat cron tool warnings as non-fatal when run recovered#94855

fix(cron): treat cron tool warnings as non-fatal when run recovered#94855
LiLan0125 wants to merge 3 commits into
openclaw:mainfrom
LiLan0125:fix/94846-cron-tool-warning-non-fatal

LiLan0125 commented Jun 19, 2026 •

edited

Loading

Uh oh!

clawsweeper Bot commented Jun 19, 2026 •

edited

Loading

Uh oh!

LiLan0125 commented Jun 19, 2026

Uh oh!

clawsweeper Bot commented Jun 19, 2026 •

edited

Loading

Uh oh!

LiLan0125 commented Jun 19, 2026

Uh oh!

clawsweeper Bot commented Jun 19, 2026 •

edited

Loading

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

1 participant

Uh oh!

Uh oh!

Conversation

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

Uh oh!

Summary

Verification

Impact Assessment

Real behavior proof

Uh oh!

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

Uh oh!

Uh oh!

LiLan0125 commented Jun 19, 2026

Uh oh!

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

Uh oh!

Uh oh!

LiLan0125 commented Jun 19, 2026

Uh oh!

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

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

1 participant

LiLan0125 commented Jun 19, 2026 •

edited

Loading

clawsweeper Bot commented Jun 19, 2026 •

edited

Loading

clawsweeper Bot commented Jun 19, 2026 •

edited

Loading

clawsweeper Bot commented Jun 19, 2026 •

edited

Loading