fix: surface cron errors for empty isolated agent replies by zhangguiping-xydt · Pull Request #100229 · openclaw/openclaw

zhangguiping-xydt · 2026-07-05T08:17:05Z

What Problem This Solves

Fixes an issue where cron-driven embedded agent runs could appear completed even when the final assistant turn produced no deliverable reply, leaving the configured channel without a response or failure signal. This complements the interactive empty-reply fix that already closed #100162.

Why This Change Was Made

Cron finalization now treats a delivery-requested embedded run with no final payload or synthesized text as a cron run error instead of passing it into the silent delivery path. The guard preserves explicit silent replies, committed message-tool deliveries, message-tool/source deliveries that already satisfied the target, and committed terminal progress such as deterministic approval prompts, accepted child sessions, or successful cron adds. Bare message-tool attempt flags without committed evidence still become errors.

User Impact

Operators and channel users now get an observable cron failure state for empty final assistant completions instead of a dark lane where the job is marked complete but no reply reaches the destination. Intentional silence and already-committed side effects remain successful.

Evidence

Added focused regressions for a completed embedded run with delivery requested and no final payload. They cover plain empty completion, an unverified message-tool attempt, explicit NO_REPLY, committed message-tool text, deterministic approval prompts, accepted child-session handoffs, and successful cron adds.
Focused Testbox proof passed: 1 file, 12 tests. https://github.com/openclaw/openclaw/actions/runs/29638298596
Near-real cron proof passed using qa-lab with qa-channel, a real gateway child process, and the mock-openai provider. The run log showed status=error, delivery=not-delivered, and one failure notification instead of a normal empty success reply.
Live cron proof passed using qa-lab with qa-channel, a real gateway child process, and live-frontier claude-cli/opus. The forced run completed through the cron path and delivered its QA marker.
pnpm tsgo:core passed.
pnpm tsgo:core:test passed.
Fresh full-branch autoreview after the maintainer fixes reported no actionable findings (confidence 0.91).

clawsweeper · 2026-07-05T08:29:36Z

Codex review: needs maintainer review before merge. Reviewed July 18, 2026, 5:30 AM ET / 09:30 UTC.

Summary
The PR makes a delivery-requested isolated cron run fail visibly when it completes without a final assistant payload, synthesized text, intentional silence, or committed terminal delivery/progress evidence, and adds regressions for those cases.

PR surface: Source +36, Tests +274. Total +310 across 2 files.

Reproducibility: yes. The source path is clear: a delivery-requested isolated cron completion with no final payload can otherwise proceed into normal delivery without content, and the focused regression suite models that path.

Review metrics: none identified.

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:

[P2] Confirm the side-effect-only success policy and review the exact merge head after required checks finish.

Risk before merge

[P1] Merging changes the success/error classification of delivery-requested cron runs that finish with no final reply; the branch intentionally preserves selected committed side-effect outcomes, so maintainers should confirm that compatibility contract before landing.

Maintainer options:

Confirm the retained success cases (recommended)
Approve the enumerated committed terminal outcomes as compatible success cases, then land once the exact merge head has satisfactory required checks.
Narrow the exemption contract
Revise the success exemptions if maintainers want more side-effect-only empty completions to surface as failures.

Next step before merge

[P2] The branch has no discrete repair finding; the remaining merge blocker is the assigned owner’s compatibility decision plus normal validation of the current merge head.

Maintainer decision needed

Question: Should a delivery-requested cron run with no final assistant reply remain successful when it has already committed messaging delivery, a deterministic approval prompt, an accepted child-session handoff, or a successful cron add?
Rationale: The patch is mechanically focused, but choosing which side-effect-only outcomes count as a successful terminal cron run is a compatibility and product-contract decision.
Likely owner: steipete — This person authored the branch’s compatibility refinement and is assigned to the PR.
Options:
- Preserve committed terminal progress (recommended): Keep the branch’s enumerated committed outcomes successful and surface only uncommitted empty completions as cron errors.
- Use a strict reply-required contract: Treat every delivery-requested completion without a final reply as an error, including side-effect-only outcomes.

Security
Cleared: The patch changes only isolated-cron finalization and its tests; it adds no dependency, permission, secret, workflow, artifact-download, or supply-chain behavior.

Review details

Best possible solution:

Keep the guard at cron finalization, preserve only explicitly committed terminal outcomes as successful, and land after the assigned owner confirms that policy and the exact merge head completes required validation.

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

Yes. The source path is clear: a delivery-requested isolated cron completion with no final payload can otherwise proceed into normal delivery without content, and the focused regression suite models that path.

Is this the best way to solve the issue?

Yes, conditionally. Cron finalization is the narrow owner boundary because it knows delivery was requested, but maintainers must confirm the branch’s listed side-effect-only success exemptions.

AGENTS.md: found and applied where relevant.

Codex review notes: model internal, reasoning high; reviewed against 5057fd9e597e.

Label changes

Label changes:

add rating: 🐚 platinum hermit: Overall readiness is 🐚 platinum hermit; proof is 🦞 diamond lobster and patch quality is 🐚 platinum hermit.
remove rating: 🦞 diamond lobster: Current PR rating is rating: 🐚 platinum hermit, so this older rating label is no longer current.

Label justifications:

P1: The patch addresses a silent unattended cron failure that can leave an active channel workflow without a response or observable error.
merge-risk: 🚨 compatibility: The patch intentionally changes whether existing no-reply cron completions are classified as success or error based on terminal side effects.
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 (live_output): The PR body provides focused Testbox coverage, qa-lab/qa-channel gateway evidence, and live cron-path output demonstrating an observable error instead of silent successful completion.
proof: sufficient: Contributor real behavior proof is sufficient. The PR body provides focused Testbox coverage, qa-lab/qa-channel gateway evidence, and live cron-path output demonstrating an observable error instead of silent successful completion.

Evidence reviewed

PR surface:

Source +36, Tests +274. Total +310 across 2 files.

View PR surface stats

Area	Files	Added	Net
Source	1	36	+36
Tests	1	274	+274
Docs	0	0	0
Config	0	0	0
Generated	0	0	0
Other	0	0	0
Total	2	310	+310

What I checked:

Current-main behavior: The inspected current-main run.ts does not contain the PR’s hasCommittedTerminalProgress or empty-final-assistant-payload guard, so current main does not yet implement this cron-specific finalization behavior. (src/cron/isolated-agent/run.ts:1445, 5057fd9e597e)
Proposed boundary and coverage: The PR adds the finalization guard after existing delivery/error handling and adds regression cases for plain empty completion, uncommitted message-tool attempts, explicit silence, committed messaging, approval prompts, accepted child sessions, and cron adds. (src/cron/isolated-agent/run.meta-error-status.test.ts:97, 1f9382130ab8)
Maintainer-directed compatibility refinement: Commit 60b3a59d added the explicit terminal-progress exemptions, making the remaining decision about the intended success contract rather than an unaddressed mechanical defect. (src/cron/isolated-agent/run.ts:1445, 60b3a59dfe39)
Behavior proof: The PR body records focused Testbox coverage plus qa-lab/qa-channel gateway proof showing status=error, delivery=not-delivered, and a failure notification for the empty-completion path; the PR carries proof: sufficient. (src/cron/isolated-agent/run.meta-error-status.test.ts:97, 1f9382130ab8)
Related work is distinct: The open related PR handles fallback selection after fallback-safe incomplete embedded results, whereas this PR determines how a completed-but-empty cron run is finalized and surfaced. (src/cron/isolated-agent/run.ts:1445, 1f9382130ab8)

Likely related people:

steipete: Authored the branch’s terminal-progress compatibility refinement and the subsequent main merges, and is assigned to this PR. (role: recent area contributor; confidence: high; commits: 60b3a59dfe39, 5b4c7f0390f6, 1f9382130ab8; files: src/cron/isolated-agent/run.ts, src/cron/isolated-agent/run.meta-error-status.test.ts)
AmirF194: Provided the concrete review of the cron finalization boundary and requested coverage for the intentional-silence/source-delivery exclusions. (role: reviewer; confidence: medium; files: src/cron/isolated-agent/run.ts, src/cron/isolated-agent/run.meta-error-status.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.

Review history (8 earlier review cycles)

reviewed 2026-07-05T08:33:48.409Z sha 8b9a18d :: needs real behavior proof before merge. :: [P2] Retire sessions before the new empty-payload return
reviewed 2026-07-05T09:31:11.927Z sha 62ac1fd :: needs real behavior proof before merge. :: none
reviewed 2026-07-05T10:37:45.418Z sha 62ac1fd :: needs real behavior proof before merge. :: none
reviewed 2026-07-05T14:36:47.575Z sha 485726e :: needs real behavior proof before merge. :: none
reviewed 2026-07-05T14:49:35.314Z sha 485726e :: needs real behavior proof before merge. :: none
reviewed 2026-07-05T18:40:30.329Z sha 485726e :: needs maintainer review before merge. :: none
reviewed 2026-07-18T09:04:30.072Z sha 60b3a59 :: needs maintainer review before merge. :: none
reviewed 2026-07-18T09:22:15.920Z sha 5b4c7f0 :: needs maintainer review before merge. :: none

AmirF194 · 2026-07-05T08:40:36Z

The core of this is right. A completed embedded run that produces no deliverable payload and no error signal really does fall through finalizeCronRun into the normal dispatchCronDelivery path with nothing to send, which is the silent dark lane you describe. Catching it as a cron error at this boundary is the correct call: cron is the only place that knows delivery was requested, interactive callers legitimately have empty replies, and there is no reply content to deliver, so surfacing an observable failure beats delivering nothing.

The guard conditions hold up. !hasFatalErrorPayload covers structured, failure-signal, and run-level errors (it ORs all three), so it cannot shadow the structured-error block below it. The terminalReplyKind !== "silent-empty" and !satisfiesSourceDelivery checks correctly preserve intentional silent replies and message-tool deliveries, and aborted empties are already handled by the earlier return, so this only fires on genuine non-aborted empty completions. The added test fails without the change (pre-fix the flow reaches the mocked dispatchCronDelivery and does not return status: "error").

Two things before merge:

Please rebase onto current main. The branch is behind, and both CI reds are base drift rather than your change: check-lint trips on src/crestodian/onboarding-welcome.ts, which is not in your diff (CI lints the merge with base), and the QA smoke failure is native-command-session-target, unrelated to cron delivery.
Consider adding a test that an empty completion with terminalReplyKind: "silent-empty" (or a satisfied source delivery) stays non-error. Right now only the error path is covered, so the exclusions that keep silent runs quiet are not guarded against regression.

Minor: the PR body's Evidence references run.empty-delivery.test.ts, which is not in the branch. The real added test is in run.meta-error-status.test.ts, worth correcting so reviewers can find it.

zhangguiping-xydt · 2026-07-05T08:43:53Z

The core of this is right. A completed embedded run that produces no deliverable payload and no error signal really does fall through finalizeCronRun into the normal dispatchCronDelivery path with nothing to send, which is the silent dark lane you describe. Catching it as a cron error at this boundary is the correct call: cron is the only place that knows delivery was requested, interactive callers legitimately have empty replies, and there is no reply content to deliver, so surfacing an observable failure beats delivering nothing.

The guard conditions hold up. !hasFatalErrorPayload covers structured, failure-signal, and run-level errors (it ORs all three), so it cannot shadow the structured-error block below it. The terminalReplyKind !== "silent-empty" and !satisfiesSourceDelivery checks correctly preserve intentional silent replies and message-tool deliveries, and aborted empties are already handled by the earlier return, so this only fires on genuine non-aborted empty completions. The added test fails without the change (pre-fix the flow reaches the mocked dispatchCronDelivery and does not return status: "error").

Two things before merge:

Please rebase onto current main. The branch is behind, and both CI reds are base drift rather than your change: check-lint trips on src/crestodian/onboarding-welcome.ts, which is not in your diff (CI lints the merge with base), and the QA smoke failure is native-command-session-target, unrelated to cron delivery.

Consider adding a test that an empty completion with terminalReplyKind: "silent-empty" (or a satisfied source delivery) stays non-error. Right now only the error path is covered, so the exclusions that keep silent runs quiet are not guarded against regression.

Minor: the PR body's Evidence references run.empty-delivery.test.ts, which is not in the branch. The real added test is in run.meta-error-status.test.ts, worth correcting so reviewers can find it.

Thanks for the review,I will take a look about the two things.

zhangguiping-xydt · 2026-07-05T14:34:31Z

@clawsweeper re-review

clawsweeper · 2026-07-05T14:34:34Z

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

zhangguiping-xydt · 2026-07-05T18:26:22Z

@clawsweeper re-review

steipete · 2026-07-18T09:41:48Z

Merged via squash.

Prepared head SHA: 1f9382130ab8c93cfccecad53067d92a8002345d
Landed commit: 1af6d99658098ceb2f75f36acf68b92c8eefe635

…00229) * fix(cron): fail empty isolated agent completions * fix: surface cron errors for empty isolated agent replies * fix(cron): preserve intentional terminal progress --------- Co-authored-by: Peter Steinberger <[email protected]> Co-authored-by: Peter Steinberger <[email protected]>

openclaw-barnacle Bot added the size: S label Jul 5, 2026

zhangguiping-xydt force-pushed the feat/issue-100162 branch from 8b9a18d to 62ac1fd Compare July 5, 2026 09:19

clawsweeper Bot removed the merge-risk: 🚨 session-state 🚨 May lose, corrupt, stale, or mis-associate session, agent, or context state. label Jul 5, 2026

zhangguiping-xydt added 2 commits July 5, 2026 20:29

fix(cron): fail empty isolated agent completions

c5b7c85

fix: surface cron errors for empty isolated agent replies

485726e

zhangguiping-xydt force-pushed the feat/issue-100162 branch from 62ac1fd to 485726e Compare July 5, 2026 14:20

zhangguiping-xydt changed the title ~~fix: embedded_run completes with empty final payload → reply silently dropped (lane goes dark~~ fix: surface cron errors for empty isolated agent replies Jul 5, 2026

openclaw-barnacle Bot added size: M and removed size: S labels Jul 5, 2026

AmirF194 mentioned this pull request Jul 6, 2026

fix(cron): retry fallback on empty embedded cron results #99911

Open

steipete self-assigned this Jul 18, 2026

fix(cron): preserve intentional terminal progress

60b3a59

clawsweeper Bot added rating: 🦞 diamond lobster Very strong PR readiness with only minor maintainer review expected. and removed rating: 🐚 platinum hermit Good normal PR readiness with ordinary maintainer review expected. labels Jul 18, 2026

steipete and others added 2 commits July 18, 2026 02:16

chore: merge main into cron empty-reply fix

5b4c7f0

Merge branch 'main' into feat/issue-100162

1f93821

clawsweeper Bot added rating: 🐚 platinum hermit Good normal PR readiness with ordinary maintainer review expected. and removed rating: 🦞 diamond lobster Very strong PR readiness with only minor maintainer review expected. labels Jul 18, 2026

steipete merged commit 1af6d99 into openclaw:main Jul 18, 2026
192 of 196 checks passed

github-actions Bot mentioned this pull request Jul 18, 2026

📡 Upstream Digest — 2026-07-18 10:56 UTC curtismercier/openclaw-mods#1333

Open

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

Uh oh!

fix: surface cron errors for empty isolated agent replies#100229

fix: surface cron errors for empty isolated agent replies#100229
steipete merged 5 commits into
openclaw:mainfrom
zhangguiping-xydt:feat/issue-100162

zhangguiping-xydt commented Jul 5, 2026 •

edited by steipete

Loading

Uh oh!

clawsweeper Bot commented Jul 5, 2026 •

edited

Loading

Uh oh!

AmirF194 commented Jul 5, 2026

Uh oh!

zhangguiping-xydt commented Jul 5, 2026

Uh oh!

zhangguiping-xydt commented Jul 5, 2026

Uh oh!

clawsweeper Bot commented Jul 5, 2026 •

edited

Loading

Uh oh!

zhangguiping-xydt commented Jul 5, 2026

Uh oh!

Uh oh!

steipete commented Jul 18, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

Uh oh!

Uh oh!

Conversation

zhangguiping-xydt commented Jul 5, 2026 • edited by steipete Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

What Problem This Solves

Why This Change Was Made

User Impact

Evidence

Uh oh!

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

Uh oh!

Uh oh!

AmirF194 commented Jul 5, 2026

Uh oh!

zhangguiping-xydt commented Jul 5, 2026

Uh oh!

zhangguiping-xydt commented Jul 5, 2026

Uh oh!

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

Uh oh!

Uh oh!

zhangguiping-xydt commented Jul 5, 2026

Uh oh!

Uh oh!

steipete commented Jul 18, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

zhangguiping-xydt commented Jul 5, 2026 •

edited by steipete

Loading

clawsweeper Bot commented Jul 5, 2026 •

edited

Loading

clawsweeper Bot commented Jul 5, 2026 •

edited

Loading