fix(cron): detect recovered tool warnings without preferFinalAssistantVisibleText by Pandah97 · Pull Request #94849 · openclaw/openclaw

Pandah97 · 2026-06-19T07:17:29Z

Summary

An isolated scheduled agentTurn that recovers from an early tool error,
produces final assistant output, and ends with status=success should still
be eligible for delivery dispatch. Instead, hasRecoveredToolWarning in
resolveCronPayloadOutcome blocked recovery and shouldUseFinalAssistantVisibleText
required preferFinalAssistantVisibleText === true, which caused channels
like Feishu (where this flag is false) to classify the recovered run as having
a fatal structured error payload, causing finalizeCronRun to return before
dispatchCronDelivery was reached.

Fix: Two changes to src/cron/isolated-agent/helpers.ts:

Removed the preferFinalAssistantVisibleText === true requirement from
hasRecoveredToolWarning so recovery detection depends on agent behavior
(final output + all errors are tool warnings), not on channel preference.
Added || hasRecoveredToolWarning to shouldUseFinalAssistantVisibleText
so that recovered runs on non-preference channels still deliver the final
assistant text instead of the stale warning payload.

Fixes #94846

Real behavior proof

Behavior or issue addressed: An isolated cron agent that hits a temporary
tool warning (e.g. ⚠️ 🛠️ get_weather: API rate limited) but then recovers
and produces valid final output was incorrectly classified as having a fatal
error on channels where preferFinalAssistantVisibleText is false (e.g.
Feishu). This caused finalizeCronRun to skip delivery dispatch entirely.

Real environment tested: Linux x64 (kernel 4.19.112), Node.js v24.13.1,
OpenClaw commit 4d019f7.

Exact steps or command run after this patch:

node --import tsx /tmp/proof-94849-v2.mjs

After-fix evidence:

=== Real Behavior Proof: PR #94849 (v2) ===
Runtime: v24.13.1 on linux x64

--- Test 1: Feishu-like channel (preferFinalAssistantVisibleText=false) ---
hasFatalErrorPayload: false
hasFatalStructuredErrorPayload: false
summary: The weather in NYC is sunny, 72°F.
outputText: The weather in NYC is sunny, 72°F.
deliveryPayloads: [{"text":"The weather in NYC is sunny, 72°F."}]

--- Test 2: Telegram-like channel (preferFinalAssistantVisibleText=true) ---
hasFatalErrorPayload: false
summary: The weather in NYC is sunny, 72°F.

--- Test 3: Tool warning without final output (still fatal) ---
hasFatalErrorPayload: true

--- Test 4: Non-tool error with final output (remains fatal) ---
hasFatalErrorPayload: true

=== Verification Summary ===
Feishu recovery detection:          PASS
Telegram no regression:             PASS
No-output still fatal:              PASS
Non-tool error still fatal:         PASS
All checks:                         PASS

Observed result after the fix: resolveCronPayloadOutcome correctly
detects recovered tool warnings regardless of preferFinalAssistantVisibleText.
Feishu-like channels now receive delivery dispatch with the final assistant
text, while non-tool errors and runs without final output remain correctly
classified as fatal. All existing Telegram-like behavior is preserved.

What was not tested: Live Feishu channel delivery. The proof validates
the resolveCronPayloadOutcome function at the runtime contract level using
the same source that production channels use.

Tests and validation

Updated the detects recovered tool warnings without preferFinalAssistantVisibleText
test to verify outputText and deliveryPayloads are correctly set
All 29 resolveCronPayloadOutcome tests pass unchanged
11 related cron run tests (payload-fallbacks + meta-error-status) pass

Risk checklist

Did user-visible behavior change? (Yes / No)
Yes — Feishu (and other channels without preferFinalAssistantVisibleText)
will now receive delivery for recovered tool-warning runs, instead of silently
skipping delivery.

Did config, environment, or migration behavior change? (Yes / No)
No

Did security, auth, secrets, network, or tool execution behavior change? (Yes / No)
No

What is the highest-risk area?

Runs where all error payloads are tool warnings (⚠️ 🛠️ prefix) but
were previously intentionally treated as fatal due to channel preference
— now they will proceed to delivery dispatch.

How is that risk mitigated?

isCronToolWarning only matches the explicit ⚠️ 🛠️ prefix, so
non-tool errors (other prefixes, plain text) remain unaffected
shouldUseFinalAssistantVisibleText still independently respects
channel preference for which text goes into the delivery payload
The recovery guard (normalizedFinalAssistantVisibleText !== undefined)
ensures the agent actually produced final output before marking run as recovered
All existing tests pass unchanged

Current review state

What is the next action?

Maintainer review

…tVisibleText - Removed preferFinalAssistantVisibleText === true requirement from hasRecoveredToolWarning so that recovery detection works for all channels (e.g. Feishu) regardless of their output preference - Added test verifying recovery detection without the channel flag Fixes openclaw#94846

clawsweeper · 2026-06-19T07:18:31Z

Codex review: needs changes before merge. Reviewed June 25, 2026, 11:24 PM ET / 03:24 UTC.

Summary
The PR removes the channel-preference requirement from recovered cron tool-warning classification, lets that recovery select final assistant text, and adds a helper regression test.

PR surface: Source -1, Tests +22. Total +21 across 2 files.

Reproducibility: yes. source-level: current main still requires the channel final-text preference before recovered tool warnings can clear fatality and select final assistant text. I did not run a live Feishu or Slack scheduled cron in this read-only review.

Review metrics: 1 noteworthy metric.

Recovered-warning resolver gates: 2 changed. The PR changes both fatality recovery and final-text delivery selection before cron dispatch, which is the user-visible risk surface.

Root-cause cluster
Relationship: fixed_by_candidate
Canonical: #94846
Summary: This PR is a candidate fix for the canonical recovered isolated-cron tool-warning delivery skip; several sibling PRs overlap but have different semantics.

Members:

canonical: [Bug]: Cron isolated agentTurn skips delivery before dispatch when a recovered early tool error is classified as fatal #94846 - The PR body uses closing syntax for this open issue and targets its recovered early tool-error delivery skip.
fixed_by_candidate: fix(cron): treat cron tool warnings as non-fatal when run recovered #94855 - This open PR also targets the canonical issue by changing recovered tool-warning nonfatal classification and final-text selection.
partial_overlap: fix(cron): allow recovered early tool errors to succeed delivery (#94846) #94862 - This open PR targets recovered early tool errors but also changes run-level-error recovery semantics.
partial_overlap: fix(cron): classify recovered agentTurn as non-fatal when finalAssistantVisibleText exists #96266 - This open PR targets a broader finalAssistantVisibleText recovery gate for a related cron misclassification report.
duplicate: Bug: isolated agentTurn cron marked error when exec exits 1, even when agent recovers and produces correct output #94863 - This closed issue reports the same recovered isolated cron tool-warning classification failure for a different channel and script shape.
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: 🐚 platinum hermit
Proof: 🦞 diamond lobster
Patch quality: 🐚 platinum hermit
Result: ready for maintainer review.

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

Rank-up moves:

Tighten the added regression so it fails on current main for the false-policy finalAssistantVisibleText-only path.

Mantis proof suggestion
A live no-preference channel smoke would materially show that recovered cron delivery sends the final assistant text instead of the stale tool-warning text. A maintainer can ask Mantis to capture proof by posting this exact PR comment:

@openclaw-mantis slack desktop smoke: verify a recovered isolated cron run delivers final assistant text instead of stale tool-warning text.

Risk before merge

[P1] The diff changes recovered cron warning runs from fatal/no-dispatch to ok/dispatch, so it affects persisted run outcome and channel delivery behavior.
[P1] The added regression currently includes a successful post-error payload, so it does not fail on current main for the exact false-policy finalAssistantVisibleText-only path.
[P1] Several open PRs target the same cron recovery cluster with different semantics, so maintainers should pick one canonical landing path before stacking fixes.

Maintainer options:

Tighten The Regression Before Merge (recommended)
Update the helper test so an error payload plus finalAssistantVisibleText and no preference flag fails on current main and passes on this PR.
Accept Helper-Level Risk
Maintainers can merge after CI if they are comfortable relying on the PR body's terminal proof despite the weak committed regression shape.
Pick A Different Canonical PR
If maintainers prefer a broader sibling recovery strategy, pause this branch and close it only after the chosen PR lands.

Copy recommended automerge instruction

@clawsweeper automerge

Special instructions:
Update the added resolveCronPayloadOutcome regression so it omits the successful post-error payload and proves an error payload plus finalAssistantVisibleText with no preference flag recovers to final text; keep production behavior unchanged unless that test exposes a bug.

Next step before merge

[P2] A repair worker can safely tighten the ineffective regression test; maintainer judgment is still needed for choosing the canonical overlapping PR.

Security
Cleared: The diff is limited to cron helper logic and a focused test, with no dependency, workflow, secret, package, permission, or code-execution surface added.

Review findings

[P3] Make the regression hit the false-policy recovery path — src/cron/isolated-agent.helpers.test.ts:61

Review details

Best possible solution:

Land one canonical narrow cron payload-outcome fix that keeps true fatal structured errors fatal, proves false-policy recovered warning delivery with a regression that fails on main, and closes the linked issue after merge.

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

Yes, source-level: current main still requires the channel final-text preference before recovered tool warnings can clear fatality and select final assistant text. I did not run a live Feishu or Slack scheduled cron in this read-only review.

Is this the best way to solve the issue?

Yes for the narrow tool-warning scope: resolveCronPayloadOutcome is the shared decision point before cron finalization dispatch. The mergeable shape should tighten the regression test and let maintainers choose the canonical branch among overlapping fixes.

Full review comments:

[P3] Make the regression hit the false-policy recovery path — src/cron/isolated-agent.helpers.test.ts:61
The new test includes a successful non-error payload after the warning, so current main already recovers through hasSuccessfulPayloadAfterLastError without this PR. Remove that payload and rely on finalAssistantVisibleText with no preference flag so the test covers the changed gate.
Confidence: 0.89

Overall correctness: patch is correct
Overall confidence: 0.84

AGENTS.md: found and applied where relevant.

Codex review notes: model internal, reasoning high; reviewed against 448b7c75b669.

Label changes

Label justifications:

P2: The PR targets a bounded cron delivery/status bug that can suppress scheduled reports but does not make the core runtime unusable.
merge-risk: 🚨 session-state: The diff changes fatal-versus-recovered cron outcome classification, which affects persisted run status and diagnostics.
merge-risk: 🚨 message-delivery: The diff changes whether recovered cron warning runs reach dispatch and which text is delivered to the channel.
rating: 🐚 platinum hermit: Overall readiness is 🐚 platinum hermit; proof is 🦞 diamond lobster and patch quality is 🐚 platinum hermit.
status: 👀 ready for maintainer look: ClawSweeper has no concrete contributor-facing blocker left for this PR. Sufficient (terminal): The PR body includes terminal output from a real Node process exercising the resolver scenarios, including the false-policy recovered-warning path and fatal controls.
proof: sufficient: Contributor real behavior proof is sufficient. The PR body includes terminal output from a real Node process exercising the resolver scenarios, including the false-policy recovered-warning path and fatal controls.

Evidence reviewed

PR surface:

Source -1, Tests +22. Total +21 across 2 files.

View PR surface stats

Area	Files	Added	Removed	Net
Source	1	1	2	-1
Tests	1	22	0	+22
Docs	0	0	0	0
Config	0	0	0	0
Generated	0	0	0	0
Other	0	0	0	0
Total	2	23	2	+21

Acceptance criteria:

[P1] node scripts/run-vitest.mjs src/cron/isolated-agent.helpers.test.ts.

What I checked:

Repository policy read: Root AGENTS.md was read fully and applied because this PR touches cron runtime/session delivery behavior and ClawSweeper review policy requires source, tests, related items, and history before verdict. (AGENTS.md:1, 448b7c75b669)
Current main still has the reported gate: Current main requires preferFinalAssistantVisibleText for both hasRecoveredToolWarning and final assistant text selection, so this PR is not obsolete on main. (src/cron/isolated-agent/helpers.ts:289, 448b7c75b669)
Cron finalization returns before dispatch on fatal structured errors: runCronIsolatedAgentTurn consumes resolveCronPayloadOutcome and returns before dispatchCronDelivery when hasFatalStructuredErrorPayload remains true and delivery was requested. (src/cron/isolated-agent/run.ts:1238, 448b7c75b669)
Direct delivery uses resolver-selected payloads: dispatchCronDelivery sends deliveryPayloads before falling back to synthesized text, so resolver selection changes the user-visible channel payload. (src/cron/isolated-agent/delivery-dispatch.ts:988, 448b7c75b669)
PR changes the central resolver gates: The PR diff removes the preference requirement from hasRecoveredToolWarning and adds hasRecoveredToolWarning to shouldUseFinalAssistantVisibleText. (src/cron/isolated-agent/helpers.ts:289, 509c7fea7f06)
Regression gap remains: The added test includes a successful non-error payload after the warning, which current main already recovers through hasSuccessfulPayloadAfterLastError, so it does not prove the false-policy finalAssistantVisibleText-only path. (src/cron/isolated-agent.helpers.test.ts:61, 509c7fea7f06)

Likely related people:

sercada: Authored the merged cron fatal completion announce suppression work that added adjacent fatal structured-error behavior in the same resolver/finalization boundary. (role: introduced adjacent fatal-dispatch behavior; confidence: high; commits: 0c7220f5da78, 2e6ca9f703da; files: src/cron/isolated-agent/helpers.ts, src/cron/isolated-agent/run.ts, src/cron/isolated-agent/run.message-tool-policy.test.ts)
steipete: Merged PR 83724 and authored follow-up commits preserving cleanup and tests around the same fatal announce suppression path. (role: merger and adjacent contributor; confidence: high; commits: 0c7220f5da78, 552e0bdf7616, 540599c1a74a; files: src/cron/isolated-agent/helpers.ts, src/cron/isolated-agent/run.ts, src/cron/isolated-agent/run.message-tool-policy.test.ts)
welfo-beo: Authored the cron final announce delivery PR that introduced finalAssistantVisibleText and the channel preference path used by this resolver. (role: adjacent feature contributor; confidence: medium; commits: 81c7304a18b8, 01bfe4f25e3f; files: src/cron/isolated-agent/helpers.ts, src/cron/isolated-agent.helpers.test.ts, src/cron/isolated-agent/run.ts)
vincentkoc: Recent commits touched isolated cron run behavior and lazy runtime boundaries around the same delivery classification surface. (role: recent area contributor; confidence: medium; commits: 28787985c4eb, 5b11985439e4, a5980df10128; files: 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.

Pandah97 · 2026-06-23T07:34:28Z

@clawsweeper re-review

clawsweeper · 2026-06-23T07:34:31Z

🦞🧹
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.

…antVisibleText hasRecoveredToolWarning already detects recovery without preferFinalAssistantVisibleText, but shouldUseFinalAssistantVisibleText still required the flag, so Feishu-like channels still got the stale warning payload instead of the final assistant text. Adding || hasRecoveredToolWarning ensures the recovery also controls which text is selected for delivery payload. Updated test assertions to verify outputText and deliveryPayloads in the no-preference case.

Pandah97 · 2026-06-26T03:13:28Z

@clawsweeper re-review

clawsweeper · 2026-06-26T03:13:31Z

🦞🧹
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.

vincentkoc · 2026-07-04T05:13:49Z

maintainer closeout after re-review against current main (85d31e10d5fc).

The ⚠️ 🛠️ prefix is presentation text, not recovery evidence. The same prefix is emitted for unresolved mutating exec/write/send failures, so removing the channel-preference guard can convert a failed mutation plus a success-looking final answer into status: ok and dispatch that answer.

The added regression also does not exercise the changed path: it includes a successful payload after the error, so current main already recovers via hasSuccessfulPayloadAfterLastError.

The July 2 report is still valid, but its sanitized evidence does not distinguish an exec preflight rejection from an actual subprocess failure. executionStarted cannot answer that today because it becomes true when tool.execute is invoked, before exec preflight and process launch.

A safe fix needs typed producer evidence, such as exec failure phase/process-started metadata, and cron must consume that metadata rather than infer semantics from display text. If an ambiguous/mutating command actually ran, the broader product fix is a first-class degraded/warning cron outcome across persistence, delivery, alerts, and retry/backoff; this helper-only patch cannot represent that safely.

Closing this candidate rather than landing behavior that can hide failed side effects. The canonical issue remains open for the typed fix.

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 the triage: needs-pr-context Candidate: external PR body lacks required problem context or evidence. label Jun 23, 2026

openclaw-barnacle Bot removed the triage: needs-pr-context Candidate: external PR body lacks required problem context or evidence. label Jun 26, 2026

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

chore: trigger CI re-run for updated PR body

509c7fe

vincentkoc self-assigned this Jul 4, 2026

vincentkoc closed this Jul 4, 2026

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

Uh oh!

fix(cron): detect recovered tool warnings without preferFinalAssistantVisibleText#94849

fix(cron): detect recovered tool warnings without preferFinalAssistantVisibleText#94849
Pandah97 wants to merge 3 commits into
openclaw:mainfrom
Pandah97:fix/issue-94846-cron-isolated-agent-skips-delivery-before-dispatch

Pandah97 commented Jun 19, 2026 •

edited

Loading

Uh oh!

clawsweeper Bot commented Jun 19, 2026 •

edited

Loading

Uh oh!

Pandah97 commented Jun 23, 2026

Uh oh!

clawsweeper Bot commented Jun 23, 2026 •

edited

Loading

Uh oh!

Pandah97 commented Jun 26, 2026

Uh oh!

clawsweeper Bot commented Jun 26, 2026 •

edited

Loading

Uh oh!

vincentkoc commented Jul 4, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

Uh oh!

Uh oh!

Conversation

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

Uh oh!

Summary

Real behavior proof

Tests and validation

Risk checklist

Current review state

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!

Pandah97 commented Jun 23, 2026

Uh oh!

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

Uh oh!

Uh oh!

Pandah97 commented Jun 26, 2026

Uh oh!

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

Uh oh!

Uh oh!

vincentkoc commented Jul 4, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

Pandah97 commented Jun 19, 2026 •

edited

Loading

clawsweeper Bot commented Jun 19, 2026 •

edited

Loading

clawsweeper Bot commented Jun 23, 2026 •

edited

Loading

clawsweeper Bot commented Jun 26, 2026 •

edited

Loading