fix(cron): emit before_agent_reply phase unconditionally for cron triggers#93535
fix(cron): emit before_agent_reply phase unconditionally for cron triggers#93535kumaxs wants to merge 3 commits into
Conversation
…ggers
The cron watchdog (createCronAgentWatchdog) depends on the
before_agent_reply execution phase to transition into the
"executing" stage and clear its pre-execution watchdog timer.
However, this phase was only emitted when a before_agent_reply
hook was registered, creating a gap:
1. runEmbeddedAgent enters the cron-trigger branch
2. hookRunner?.hasHooks("before_agent_reply") returns false
(no hook plugin registered)
3. before_agent_reply phase is never emitted
4. The watchdog never transitions out of "waiting_for_execution"
5. Pre-execution timeout fires after 60s → runAbortController.abort()
6. The on-going LLM call is interrupted → externalAbort: true
→ "LLM request failed."
The fix unconditionally emits before_agent_reply for cron triggers,
then wraps the hook execution in the hasHooks check separately.
This ensures the watchdog sees the phase even when no hook is
registered.
Fixes openclaw#93530
openclaw-barnacle
Bot
added
agents
|
@clawsweeper review |
|
🦞🧹 I asked ClawSweeper to review this item again. |
|
Codex review: needs real behavior proof before merge. Reviewed July 1, 2026, 12:32 AM ET / 04:32 UTC. Summary PR surface: Source +8. Total +8 across 1 file. Reproducibility: yes. Current main still gates Review metrics: none identified. Merge readiness Overall follows the weaker of proof and patch quality, so missing proof can cap an otherwise strong patch. Rank-up moves:
Proof guidance:
Risk before merge
Maintainer options:
Next step before merge
Security Review findings
Review detailsBest possible solution: Settle one canonical cron runner/watchdog phase contract that fixes no-hook false aborts while preserving true startup-stall detection, then add focused regression coverage and redacted after-fix cron proof. Do we have a high-confidence way to reproduce the issue? Yes. Current main still gates Is this the best way to solve the issue? No. This is a plausible narrow mitigation, but the safer fix must preserve true startup-stall detection instead of clearing the short guard before later startup progress is emitted. Full review comments:
Overall correctness: patch is incorrect AGENTS.md: found and applied where relevant. Codex review notes: model internal, reasoning high; reviewed against 9f98b6e174d1. Label changesLabel justifications:
Evidence reviewedPR surface: Source +8. Total +8 across 1 file. View PR surface stats
What I checked:
Likely related people:
What the crustacean ranks mean
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
Bot
added
rating: 🧂 unranked krab
Real Behavior Evidence (Before-Fix)I'm tracking the opencode-go isolated cron streaming hang issue (#93530 / #93610 / #95530) and hit the exact symptom this PR targets. Sharing 3-run evidence from a single, unchanged cron job — same prompt, same model, same code, no manual intervention. Cron:
Key observations:
For full transcript comparison see local session files:
|
Additional Context: Relationship with #93914 / #93965The before-fix evidence above (run #1: Since opening #93535, two sibling PRs have landed:
The codex review identified a P2 risk: early Request for maintainers: Given that #93914 has merged, what is the recommended path for this PR? Should it:
The before-fix evidence is here and ready to use. Happy to adjust this PR or close in favor of the merged fixes — just need guidance. |
|
@clawsweeper review |
openclaw-barnacle
Bot
added
the
triage: needs-pr-context
Real Behavior Evidence (Before-Fix, Sanitized)I'm tracking the opencode-go isolated cron streaming hang issue (#93530 / #93610 / #95530) and hit the exact symptom this PR targets. Sharing redacted evidence from a single, unchanged cron job — same prompt, same model, same code, no manual intervention. Environment:
Run #1 — cron trigger, FAIL
Critical observation: Run #2 — manual retry, SUCCESS (control)
Observation: Same cron payload, same model, same code. Manual retry completed in under 3 minutes with normal token consumption. Proves the model and config are functional — the failure is not deterministic. Run #3 — cron trigger retry, SUCCESS (control)
Observation: Second cron trigger retry completed cleanly in 1m17s. The same cron that failed in run #1 completed successfully in run #3 with the same code/config. This rules out deterministic causes (config, prompt, model behavior) and points to the intermittent watchdog race. Summary
Interpretation:
If maintainers need the raw session trajectory, the redacted JSONL files (with internal IDs removed) are available on request. |
Multiple Timeout Failures (Real Behavior Evidence)补充之前 6/23 单次失败证据。以下是多个历史 cron run timeout 失败的真实记录（已脱敏）： Environment:
Failed Cron Runs
Pattern Across All FailuresAll four timeout failures share the exact same Combined with Earlier EvidenceCombined with my earlier single-run observation (
Sanitization
Earlier Single-Run Failure (already posted)From the previous comment, run at 2026-06-23T01:00:00Z:
Together, this is 5 documented timeout failures across 17 days, all hitting the same |
Sanitized Failure Trajectories5 sanitized JSONL files uploaded to GitHub Gist:
Sanitization applied
Key evidence in these filesThe
The four cron run files all show the same pattern:
Together this demonstrates 5 documented timeout/aborted failures over 17 days (May 16 - June 23), all hitting the same |
|
@clawsweeper review |
|
🦞🧹 I asked ClawSweeper to review this item again. |
clawsweeper
Bot
added
status: 🛠️ actively grinding
clawsweeper
Bot
added
status: 📣 needs proof
|
This pull request has been automatically marked as stale due to inactivity. |
1 participant
Problem
Cron isolated-agent turns can abort with
externalAbort: true,stopReason: "aborted", anderror: "LLM request failed."despite the model provider being healthy and no rate-limit hit. The abort fires after ~6 minutes, well within the configuredtimeoutSeconds: 600(10 min) wall clock.See #93530 for full issue details.
Root Cause
The cron watchdog (
createCronAgentWatchdoginsrc/cron/service/agent-watchdog.ts) depends on thebefore_agent_replyexecution phase to transition into the "executing" stage and clear its pre-execution watchdog timer.In
runEmbeddedAgentInternal(src/agents/embedded-agent-runner/run.ts), thebefore_agent_replyphase was only emitted whenhookRunner?.hasHooks("before_agent_reply")returned true. When no hook plugin registered for that event:runEmbeddedAgententers the cron-trigger branchhookRunner?.hasHooks("before_agent_reply")returns false → phase is skippedtriggerTimeout()→runAbortController.abort(reason)externalAbort: true→ "LLM request failed."Fix
This PR unconditionally emits the
before_agent_replyphase for cron triggers, then separately wraps the hook execution in thehasHooksguard. This way the watchdog sees the phase even when no before_agent_reply hook is registered.Before:
if (trigger === "cron" && hookRunner?.hasHooks("before_agent_reply"))After:
if (trigger === "cron")(emits phase) +if (hookRunner?.hasHooks("before_agent_reply"))(runs hook)Testing
src/cron/service/agent-watchdog.test.tsFixes #93530