Skip to content

fix(cron): avoid busy-wait drift for recurring main jobs#58872

Merged
scoootscooob merged 1 commit into
openclaw:mainfrom
scoootscooob:codex/fix-58833-cron-drift
Apr 1, 2026
Merged

fix(cron): avoid busy-wait drift for recurring main jobs#58872
scoootscooob merged 1 commit into
openclaw:mainfrom
scoootscooob:codex/fix-58833-cron-drift

Conversation

@scoootscooob

Copy link
Copy Markdown
Member

Summary

  • Problem: recurring main-session cron jobs with --wake now stay inside the cron run while runHeartbeatOnce keeps returning requests-in-flight, which inflates lastDurationMs and lets schedule drift accumulate.
  • Why it matters: short recurring jobs like */3 * * * * can appear to run for minutes and miss later slots even with --exact.
  • What changed: recurring main-session jobs now fall back to requestHeartbeatNow() immediately after the first busy runHeartbeatOnce result instead of busy-waiting in the cron lane.
  • What did NOT change (scope boundary): one-shot --at jobs still keep the existing synchronous wait-for-heartbeat behavior.

Change Type (select all)

  • Bug fix
  • Feature
  • Refactor required for the fix
  • Docs
  • Security hardening
  • Chore/infra

Scope (select all touched areas)

  • Gateway / orchestration
  • Skills / tool execution
  • Auth / tokens
  • Memory / storage
  • Integrations
  • API / contracts
  • UI / DX
  • CI/CD / infra

Linked Issue/PR

Root Cause / Regression History (if applicable)

  • Root cause: executeMainSessionCronJob() retried runHeartbeatOnce() for up to the busy-wait window even for recurring jobs, so the cron run duration included main-lane contention instead of just cron scheduling work.
  • Missing detection / guardrail: there was coverage for one-shot wakeMode=now behavior, but no regression test for recurring main-session jobs under requests-in-flight pressure.
  • Prior context (git blame, prior PR, issue, or refactor if known): Unknown.
  • Why this regressed now: under heavier gateway load, recurring cron jobs can hit the busy main lane path repeatedly, which magnifies duration inflation and drift.
  • If unknown, what was ruled out: the per-job cron stagger path and post-run duration math were not the primary cause here.

Regression Test Plan (if applicable)

  • Coverage level that should have caught this:
    • Unit test
    • Seam / integration test
    • End-to-end test
    • Existing coverage already sufficient
  • Target test or file: src/cron/service.issue-regressions.test.ts
  • Scenario the test should lock in: a recurring main-session cron job with wakeMode="now" and runHeartbeatOnce() returning requests-in-flight should request a queued heartbeat immediately and complete with a short recorded duration.
  • Why this is the smallest reliable guardrail: it exercises the exact scheduler path without requiring broader gateway/runtime setup.
  • Existing test that already covers this (if any): src/cron/service.runs-one-shot-main-job-disables-it.test.ts covers one-shot wakeMode now behavior, but not recurring jobs.
  • If no new test is added, why not: N/A

User-visible / Behavior Changes

Recurring main-session cron jobs using --wake now no longer sit in a long busy-wait loop when the main lane is already busy. Their cron run finishes quickly after queueing the immediate heartbeat, which keeps lastDurationMs closer to scheduler overhead and reduces drift from blocked cron slots.

Diagram (if applicable)

Before:
[recurring cron due] -> [enqueue system event] -> [busy runHeartbeatOnce loop] -> [minutes counted as cron duration] -> [later slots drift]

After:
[recurring cron due] -> [enqueue system event] -> [first busy runHeartbeatOnce] -> [requestHeartbeatNow] -> [cron run completes quickly]

Security Impact (required)

  • New permissions/capabilities? (Yes/No) No
  • Secrets/tokens handling changed? (Yes/No) No
  • New/changed network calls? (Yes/No) No
  • Command/tool execution surface changed? (Yes/No) No
  • Data access scope changed? (Yes/No) No
  • If any Yes, explain risk + mitigation:

Repro + Verification

Environment

  • OS: macOS
  • Runtime/container: local test runner
  • Model/provider: N/A
  • Integration/channel (if any): N/A
  • Relevant config (redacted): recurring main-session cron job with wakeMode=now

Steps

  1. Create a recurring main-session cron job with wakeMode=now.
  2. Make runHeartbeatOnce() return { status: "skipped", reason: "requests-in-flight" }.
  3. Execute the job and inspect the recorded result.

Expected

  • The cron job should enqueue the event, request an immediate heartbeat, and finish quickly.

Actual

  • Before this change, the cron job stayed in the busy-wait retry loop and recorded an inflated duration.

Evidence

  • Failing test/log before + passing after
  • Trace/log snippets
  • Screenshot/recording
  • Perf numbers (if relevant)

Human Verification (required)

  • Verified scenarios: ran pnpm test -- src/cron/service.issue-regressions.test.ts -t "#58833|busy-recurring-main|main lane is busy" and pnpm test -- src/cron/service.runs-one-shot-main-job-disables-it.test.ts -t "wakeMode now".
  • Edge cases checked: confirmed the existing one-shot wakeMode now tests still pass, so the synchronous one-shot behavior remains intact.
  • What you did not verify: full repo-wide pnpm check is currently failing due to unrelated existing tsgo errors outside the cron surface.

Review Conversations

  • I replied to or resolved every bot review conversation I addressed in this PR.
  • I left unresolved only the conversations that still need reviewer or maintainer judgment.

Compatibility / Migration

  • Backward compatible? (Yes/No) Yes
  • Config/env changes? (Yes/No) No
  • Migration needed? (Yes/No) No
  • If yes, exact upgrade steps:

Risks and Mitigations

  • Risk: recurring main-session jobs now stop waiting sooner when the main lane is busy, so their cron run status reflects successful queueing rather than completed main-lane processing.
    • Mitigation: the change is limited to recurring jobs; one-shot jobs keep the existing wait-for-heartbeat behavior, and the new regression test locks in the intended fallback path.

@openclaw-barnacle openclaw-barnacle Bot added size: S maintainer Maintainer-authored PR labels Apr 1, 2026
@scoootscooob
scoootscooob merged commit 10750fb into openclaw:main Apr 1, 2026
36 of 43 checks passed
@greptile-apps

greptile-apps Bot commented Apr 1, 2026

Copy link
Copy Markdown
Contributor

Greptile Summary

This PR fixes a busy-wait drift issue for recurring main-session cron jobs using wakeMode="now". Previously, when the main lane was busy (runHeartbeatOnce returned requests-in-flight), the cron runner would spin inside the retry loop for up to wakeNowHeartbeatBusyMaxWaitMs (default 2 minutes), inflating lastDurationMs and causing schedule drift for short recurring jobs like */3 * * * *. The fix introduces an isRecurringJob guard (schedule.kind !== "at") that breaks out of the busy-wait on the very first busy response, falls back to requestHeartbeatNow, and returns immediately with status: "ok". One-shot at jobs retain the existing synchronous wait behavior. A dedicated regression test is added that validates exactly one runHeartbeatOnce call, the correct requestHeartbeatNow invocation, and a sub-100ms recorded duration.

  • timer.ts: Adds isRecurringJob flag and an early-exit branch for recurring jobs on the first requests-in-flight result
  • service.issue-regressions.test.ts: Adds a targeted regression test (#58833) using executeJob to assert short duration and correct heartbeat delegation
  • The isRecurringJob predicate correctly covers both cron and every schedule kinds (all non-at schedules), consistent with the existing CronSchedule union type
  • requestHeartbeatNow is a required dependency in CronServiceDeps, so calling it without a null guard here is consistent with the rest of the function

Confidence Score: 5/5

Safe to merge — the fix is well-scoped, correctly targets only recurring main-session jobs, and is guarded by a dedicated regression test.

All findings are P2 or lower. The logic change is minimal and correct: the isRecurringJob predicate covers all recurring schedule kinds (cron and every), one-shot at jobs are unaffected, and requestHeartbeatNow is a required dependency so no null-safety gap is introduced. The regression test adequately locks in the intended behavior.

No files require special attention.

Reviews (1): Last reviewed commit: "Cron: avoid busy-wait drift for recurrin..." | Re-trigger Greptile

lovewanwan pushed a commit to lovewanwan/openclaw that referenced this pull request Apr 28, 2026
ogt-redknie pushed a commit to ogt-redknie/OPENX that referenced this pull request May 2, 2026
github-actions Bot pushed a commit to Desicool/openclaw that referenced this pull request May 9, 2026
github-actions Bot pushed a commit to Desicool/openclaw that referenced this pull request May 24, 2026
Nachx639 pushed a commit to Nachx639/clawdbot that referenced this pull request Jun 17, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

maintainer Maintainer-authored PR size: S

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[Bug]: Cron jobs are not executing as scheduled.

1 participant