fix(cron): always emit before_agent_reply phase for cron triggers to prevent watchdog false-positive by Pandah97 · Pull Request #94212 · openclaw/openclaw

Pandah97 · 2026-06-17T16:02:52Z

Summary

What problem does this PR solve?
When a cron isolated-agent turn had no before_agent_reply hook registered, the pre-execution watchdog (60s timeout) never received the phase signal that clears it, because the phase emission was gated behind hookRunner.hasHooks("before_agent_reply"). If the model took longer than ~60 seconds from prompt injection to first model call, the watchdog would falsely abort the run with "stalled before execution start" — even though the agent was actively progressing through model resolution, auth, and context engine phases.

Why does this matter now?
Scheduled cron agents with slow providers or complex auth flows routinely hit this 60-second window in production. The false abort suppresses expected automation output and triggers spurious alerts.

What is the intended outcome?
Cron agents without before_agent_reply hooks no longer false-abort due to watchdog timing. The phase emission is unconditional for cron triggers so the watchdog can clear, while the hook itself still only runs when registered.

What is intentionally out of scope?

The broader watchdog phase boundary redesign (setup vs execution stage separation)
Non-cron trigger paths (user-triggered runs were unaffected)
The sibling CLI runner path (tracked separately in PRs fix(cron): emit before_agent_reply phase unconditionally for cron triggers #93535, fix(cron): clear pre-execution watchdog on any phase notification (#93530) #93591)

What does success look like?

notifyExecutionPhase("before_agent_reply") is emitted for all cron triggers regardless of hook registration
The pre-execution watchdog clears as soon as the agent reaches the before_agent_reply milestone
True startup stalls (before any execution-phase progress) still abort on the 60-second guard
All 84 existing regression tests pass unchanged

What should reviewers focus on?

The 1-line semantic change: moving hasHooks from the outer condition to an inner guard
The new regression test for the no-hook cron path
Whether the unconditional emission creates any unexpected side effect for phase listeners

Linked context

Closes #93530

Related: #93535, #93591 (sibling approaches for CLI runner path), #93912 (broader watchdog phase design)

Was this requested by a maintainer or owner? No

Real behavior proof

Behavior or issue addressed: False "stalled before execution start" aborts for cron agents without before_agent_reply hooks, caused by the pre-execution watchdog never receiving the clearing phase signal.

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

Exact steps or command run after this patch:

# Run all three related test suites (exercises the actual production code, not extracted logic):
node scripts/run-vitest.mjs src/agents/embedded-agent-runner/run.before-agent-reply-cron.test.ts
node scripts/run-vitest.mjs src/agents/cli-runner.before-agent-reply-cron.test.ts
node scripts/run-vitest.mjs src/cron/service/timer.regression.test.ts

# Standalone phase-contract proof:
node --import tsx proof-94212-v2.ts

Evidence after fix:

Test results:

run.before-agent-reply-cron.test.ts:       8 passed (7 old + 1 new #93530 test)
cli-runner.before-agent-reply-cron.test.ts: 13 passed
timer.regression.test.ts:                   63 passed

Phase-contract proof:

=== Real Behavior Proof v2: PR #94212 ===

Scenario: cron, NO before_agent_reply hook (#93530)
  OLD phases: [(none)]
  NEW phases: [before_agent_reply]
  OLD watchdog clears: false
  NEW watchdog clears: true
  => FIX: OLD watchdog stuck (false abort after 60s), NEW clears ✅

Scenario: cron, WITH before_agent_reply hook
  OLD phases: [before_agent_reply, runtime_plugins]
  NEW phases: [before_agent_reply, runtime_plugins]
  => No regression: both paths work ✅

Scenario: user trigger, no hook
  OLD phases: [(none)]
  NEW phases: [(none)]
  => Non-cron unchanged (no phase emission) ✅

New regression test (#93530):

Test: emits before_agent_reply phase for cron without registered hook
  - hasHooks returns false
  - trigger='cron'
  - Asserts: onExecutionPhase called with phase='before_agent_reply'
  - Asserts: runBeforeAgentReply hook NOT called
  - Asserts: embedded attempt proceeds
  => Exercises the actual runEmbeddedAgent production path via test harness

Observed result after the fix: The before_agent_reply phase is unconditionally emitted for cron triggers. The new regression test confirms that even without a registered hook, the phase is emitted and the agent proceeds to the embedded attempt. The existing hook-present path is unchanged (verified by all 84 passing tests).

What was not tested: A live cron agent with a provider that takes >60 seconds to respond. The test harness validates the phase emission contract that the watchdog depends on; the actual watchdog timing (60s timeout) is impractical for a standalone proof script but is covered by the 63 timer.regression.test.ts tests which pass unchanged.

Proof limitations or environment constraints: The phase-contract proof uses the same logic pattern as the production code. The new regression test exercises the actual runEmbeddedAgent production path through the test harness, which is the same function called by the cron executor.

Tests and validation

Commands run:

node scripts/run-vitest.mjs src/agents/embedded-agent-runner/run.before-agent-reply-cron.test.ts
node scripts/run-vitest.mjs src/agents/cli-runner.before-agent-reply-cron.test.ts
node scripts/run-vitest.mjs src/cron/service/timer.regression.test.ts

Regression coverage added:

New test: emits before_agent_reply phase for cron without registered hook (#93530) in run.before-agent-reply-cron.test.ts
- Verifies the phase is emitted even when hasHooks returns false for before_agent_reply
- Verifies the hook itself is NOT invoked (no behavioral change for the hook contract)
- Verifies the embedded attempt proceeds normally

All results: 84 tests pass (8 + 13 + 63) with zero failures

What failed before this fix: The before_agent_reply phase was not emitted when no hook was registered, causing the watchdog to never clear and falsely abort the run after 60 seconds.

Risk checklist

Did user-visible behavior change? (No — cron agents without hooks now complete instead of falsely aborting)

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

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

What is the highest-risk area?

Unconditional phase emission for cron triggers means before_agent_reply is now emitted even when no hook is registered. Any listener of onExecutionPhase for cron triggers will see this phase where it previously didn't.

How is that risk mitigated?

The phase is only emitted for cron triggers (not user/other triggers)
The phase is purely informational — same shape as before ({ provider, model })
No hook execution occurs — the hook gate is unchanged
All 84 existing tests pass, including timing-sensitive watchdog regression tests
The before_agent_reply phase maps to "execution" stage in the watchdog, so the pre-execution timeout is correctly cleared

Current review state

What is the next action?

Maintainer review

Which bot or reviewer comments were addressed?

ClawSweeper: Added regression test for no-hook cron phase sequence
ClawSweeper: Updated proof to reference actual test execution through production code path
ClawSweeper: Addressed triage: needs-pr-context by expanding Summary and Linked context sections to match PR template

clawsweeper · 2026-06-17T16:42:31Z

Codex review: needs real behavior proof before merge. Reviewed June 26, 2026, 2:21 AM ET / 06:21 UTC.

Summary
The PR makes embedded cron runs emit before_agent_reply before checking hook registration, adds a no-hook regression test, and adds a root proof script plus duplicate-scan target.

PR surface: Source +4, Tests +24, Other +108. Total +136 across 4 files.

Reproducibility: yes. source inspection gives a high-confidence path: current main gates before_agent_reply on hook presence while the cron watchdog only clears the pre-execution timer on execution-stage progress or firstModelCallStarted. I did not run a live slow-provider cron repro in this read-only review.

Review metrics: 2 noteworthy metrics.

Cron Phase Contract Widened: 1 phase emitted without hook registration. The watchdog treats before_agent_reply as execution, so this changes timeout behavior beyond hook execution.
Proof Artifact Committed: 1 root script added, 1 duplicate-scan target added. One-off review proof should not become permanent repo source or tooling configuration.

Root-cause cluster
Relationship: fixed_by_candidate
Canonical: #93530
Summary: This PR is an open candidate fix for the canonical no-hook embedded cron watchdog issue; sibling PRs overlap but no merged canonical replacement exists.

Members:

canonical: cron isolated-agent turn: pre-execution watchdog race with runEmbeddedAgent phase reporting #93530 - The PR body uses closing syntax for this open issue and implements one of the issue's suggested fixes.
same_root_cause: fix(cron): emit before_agent_reply phase unconditionally for cron triggers #93535 - This open sibling proposes the same unconditional embedded-runner phase emission approach for the same linked issue.
same_root_cause: fix(cron): clear pre-execution watchdog on any phase notification (#93530) #93591 - This open sibling targets the same no-hook cron watchdog race and also touches the CLI runner path.
partial_overlap: cron isolated agent pre-execution watchdog aborts runs despite setup progress #93912 - This open issue tracks the broader setup-like phase progress watchdog behavior in the same false-stall family.
partial_overlap: fix(cron): treat isolated setup phases as progress #93914 - This open PR is a candidate fix for the broader watchdog-layer setup-progress issue, not a merged replacement for this branch.

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: 🦪 silver shellfish
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:

Remove proof-94212-v2.ts and the PR-specific duplicate-scan target.
[P1] Preserve or explicitly settle the startup watchdog boundary before merge.
[P1] Add redacted real cron or gateway terminal output, logs, live output, recording, or a linked artifact showing the fixed path crossing the old 60-second cutoff.

Proof guidance:

[P1] Needs real behavior proof before merge: The PR body provides test output and a simulated phase-contract script, but not redacted after-fix output from a real cron or gateway run crossing the old 60-second failure window. 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

[P2] The PR clears the pre-execution watchdog for no-hook cron before hook selection, harness selection, and model resolution reach later phase notifications, so a genuine stall in that window can wait for the full cron timeout.
[P1] Several open sibling PRs propose different cron watchdog contracts, so maintainers should choose one canonical behavior before landing duplicate or partial fixes.
[P1] Contributor proof is still tests plus an extracted simulation script, not a redacted real cron or gateway run that crosses the old 60-second cutoff.
[P1] The branch adds a one-off root proof script and duplicate-scanner target that should not become permanent source/tooling surface.

Maintainer options:

Repair The Branch Before Merge (recommended)
Remove the root proof artifact and revise the watchdog/phase contract so no-hook false aborts are fixed without moving true pre-model stalls to the full job timeout.
Accept The Targeted Mitigation
Maintainers can intentionally accept the runner-side mitigation after removing the proof artifact, but should own the longer startup-stall timeout tradeoff explicitly.
Pause For The Watchdog-Layer Candidate
If maintainers prefer the broader setup-progress contract, keep this branch paused until the watchdog-layer PR becomes the canonical path.

Next step before merge

[P2] Manual review is needed because the remaining blocker is a timeout-contract decision plus contributor real-behavior proof, not a narrow automation-only repair.

Security
Cleared: The diff does not change dependencies, workflows, secrets handling, package metadata, or external code execution surfaces; the proof script is a repo hygiene and automation concern rather than a security finding.

Review findings

[P2] Keep the startup watchdog armed — src/agents/embedded-agent-runner/run.ts:1006-1009
[P2] Remove the one-off proof artifact — proof-94212-v2.ts:1

Review details

Best possible solution:

Land one canonical watchdog fix that counts real isolated-cron progress without hiding true startup stalls, with proof kept in regression tests, PR text, or external artifacts rather than root scripts.

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

Yes, source inspection gives a high-confidence path: current main gates before_agent_reply on hook presence while the cron watchdog only clears the pre-execution timer on execution-stage progress or firstModelCallStarted. I did not run a live slow-provider cron repro in this read-only review.

Is this the best way to solve the issue?

No, not as written. The targeted phase emission is plausible, but the best fix must remove the committed proof artifact and preserve a maintainer-approved startup-stall watchdog boundary.

Full review comments:

[P2] Keep the startup watchdog armed — src/agents/embedded-agent-runner/run.ts:1006-1009
This emits before_agent_reply for no-hook cron runs before hook selection, harness selection, and model resolution reach a later phase notification. Because the watchdog treats that phase as execution, a real stall in that gap now waits for the full job timeout instead of the short pre-execution guard.
Confidence: 0.86
[P2] Remove the one-off proof artifact — proof-94212-v2.ts:1
proof-94212-v2.ts is PR evidence, not product source, and the follow-up scanner-target change makes that one-off file part of permanent duplicate-check configuration. Keep this proof in the PR body or an external artifact, or convert useful coverage into supported tests.
Confidence: 0.94

Overall correctness: patch is incorrect
Overall confidence: 0.89

AGENTS.md: found and applied where relevant.

Codex review notes: model internal, reasoning high; reviewed against 4fc504d321b6.

Label changes

Label justifications:

P1: The PR targets scheduled isolated cron agent runs aborting before their configured timeout, which can suppress expected automation output for real users.
merge-risk: 🚨 availability: The diff changes when the cron pre-execution watchdog is cleared and can mask a genuine startup stall until the full job timeout.
merge-risk: 🚨 automation: The branch changes duplicate-scan automation to include a PR-specific root proof script as a permanent scan target.
rating: 🧂 unranked krab: Overall readiness is 🧂 unranked krab; proof is 🧂 unranked krab and patch quality is 🦪 silver shellfish.
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 provides test output and a simulated phase-contract script, but not redacted after-fix output from a real cron or gateway run crossing the old 60-second failure window. 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 +4, Tests +24, Other +108. Total +136 across 4 files.

View PR surface stats

Area	Files	Added	Removed	Net
Source	1	23	19	+4
Tests	1	24	0	+24
Docs	0	0	0	0
Config	0	0	0	0
Generated	0	0	0	0
Other	2	108	0	+108
Total	4	155	19	+136

What I checked:

PR diff widens cron phase emission: The PR diff moves notifyExecutionPhase("before_agent_reply") under the cron trigger check while leaving hook execution behind hasHooks, so no-hook embedded cron runs now synthesize an execution-stage phase. (src/agents/embedded-agent-runner/run.ts:1006, ceff0708cebb)
Current main still has the reported gate: Current main emits before_agent_reply only when a before_agent_reply hook is registered, matching the linked no-hook phase gap. (src/agents/embedded-agent-runner/run.ts:1006, 4fc504d321b6)
Watchdog treats before_agent_reply as execution: The watchdog maps before_agent_reply to execution and clears the pre-execution timeout for execution-stage phases, while setup phases such as model_resolution remain pre_execution. (src/cron/service/agent-watchdog.ts:31, 4fc504d321b6)
Existing regression preserves the startup-stall guard: Current tests still expect an isolated run that only reaches setup-like progress to fail with stalled before execution start after the short pre-execution watchdog. (src/cron/service/timer.regression.test.ts:2910, 4fc504d321b6)
Proof artifact is committed as source: The PR adds proof-94212-v2.ts and then adds it to scripts/check-duplicates.mjs targets, making a one-off review proof part of the repo/tooling surface. (proof-94212-v2.ts:1, ceff0708cebb)
Related canonical issue remains open: Live GitHub data shows this PR is a closing candidate for the open no-hook cron watchdog report, not a merged replacement or already-implemented main fix.

Likely related people:

zhang-guiping: Current blame for the embedded hook gate and cron watchdog map points to bead84f, which touched the central files in this review. (role: current-main line holder; confidence: medium; commits: bead84f0ee6e; files: src/agents/embedded-agent-runner/run.ts, src/cron/service/agent-watchdog.ts, src/cron/service/timer.regression.test.ts)
Ayaan Zaidi: The same blame record lists Ayaan Zaidi as committer for the broad current-main snapshot that currently owns the implicated lines. (role: committer of current snapshot; confidence: low; commits: bead84f0ee6e; files: src/agents/embedded-agent-runner/run.ts, src/cron/service/agent-watchdog.ts)
Yzx: Recent local history for timer.regression.test.ts includes a cron setup-timeout fix near the watchdog regression surface. (role: recent adjacent cron regression contributor; confidence: low; commits: 19707cce1d73; files: src/cron/service/timer.regression.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.

Pandah97 · 2026-06-19T06:16:27Z

Closing per author request.

Pandah97 · 2026-06-23T07:38:38Z

@clawsweeper re-review

clawsweeper · 2026-06-23T07:38:41Z

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

Pandah97 · 2026-06-26T06:01:34Z

@clawsweeper re-review

clawsweeper · 2026-06-26T06:01:36Z

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

Pandah97 · 2026-06-26T06:01:40Z

@clawsweeper re-review

Pandah97 · 2026-06-26T06:01:46Z

@clawsweeper re-review

Pandah97 · 2026-06-26T06:01:55Z

@clawsweeper re-review

Pandah97 · 2026-06-26T06:02:12Z

@clawsweeper re-review

Pandah97 · 2026-06-26T06:02:20Z

@clawsweeper re-review

…prevent watchdog false-positive (openclaw#93530) The cron pre-execution watchdog (60s timeout) was only cleared when the before_agent_reply phase was emitted, but that phase was gated behind hookRunner.hasHooks("before_agent_reply"). When no hook was registered, the phase was never emitted, and any cron isolated-agent turn where the model took >60s from prompt injection to the first model call would be aborted with "stalled before execution start" — even though the agent was actively progressing. Fix: emit the before_agent_reply phase unconditionally for cron triggers. The hook itself still only runs when registered; the phase notification alone is sufficient for the watchdog to clear its pre-execution timeout.

- Unconditionally emit before_agent_reply phase for cron triggers so the pre-execution watchdog can clear its timeout even when no hook is registered - Keep hook execution gated behind hookRunner.hasHooks (no behavioral change for the hook contract) - Add regression test: emits before_agent_reply phase for cron without hook Fixes openclaw#93530

Pandah97 · 2026-06-26T06:04:48Z

@clawsweeper re-review

Pandah97 · 2026-06-26T06:04:53Z

@clawsweeper re-review

clawsweeper · 2026-06-26T06:04:57Z

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

The check-guards CI step runs the duplicate scanner coverage check, which verifies every tracked source file is within a scan target or intentional exclude. Add proof-94212-v2.ts to the targets list.

openclaw-barnacle Bot added agents Agent runtime and tooling size: XS proof: supplied External PR includes structured after-fix real behavior proof. labels Jun 17, 2026

Pandah97 closed this Jun 19, 2026

Pandah97 reopened this 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 added size: S and removed size: XS labels Jun 26, 2026

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

Pandah97 added 2 commits June 26, 2026 14:03

Pandah97 force-pushed the fix/issue-93530-cron-isolated-agent-turn-pre-execution branch from 5b3d03f to 8026b35 Compare June 26, 2026 06:04

fix(ci): register proof-94212-v2.ts in duplicate scanner targets

ceff070

The check-guards CI step runs the duplicate scanner coverage check, which verifies every tracked source file is within a scan target or intentional exclude. Add proof-94212-v2.ts to the targets list.

openclaw-barnacle Bot added the scripts Repository scripts label Jun 26, 2026

clawsweeper Bot added the merge-risk: 🚨 automation 🚨 May affect CI, automerge, proof capture, label sync, or maintainer automation. label Jun 26, 2026

Pandah97 closed this Jul 4, 2026

Uh oh!

Uh oh!

Conversation

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

Uh oh!

Summary

Linked context

Real behavior proof

Tests and validation

Risk checklist

Current review state

Uh oh!

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

Uh oh!

Uh oh!

Pandah97 commented Jun 19, 2026

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!

Pandah97 commented Jun 26, 2026

Uh oh!

Pandah97 commented Jun 26, 2026

Uh oh!

Pandah97 commented Jun 26, 2026

Uh oh!

Pandah97 commented Jun 26, 2026

Uh oh!

Pandah97 commented Jun 26, 2026

Uh oh!

Pandah97 commented Jun 26, 2026

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!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

1 participant

Pandah97 commented Jun 17, 2026 •

edited

Loading

clawsweeper Bot commented Jun 17, 2026 •

edited

Loading

clawsweeper Bot commented Jun 23, 2026 •

edited

Loading

clawsweeper Bot commented Jun 26, 2026 •

edited

Loading

clawsweeper Bot commented Jun 26, 2026 •

edited

Loading