fix: cron race condition - run due jobs before recomputing nextRunAtMs (#9661) by divol89 · Pull Request #9684 · openclaw/openclaw

divol89 · 2026-02-05T15:26:16Z

Problem

When the cron timer fires even 1ms after the scheduled time, jobs were being systematically skipped due to a race condition in operation order.

Previous flow:

ensureLoaded → calls recomputeNextRuns → advances nextRunAtMs to NEXT occurrence
runDueJobs → checks now >= nextRunAtMs → false (already advanced) → job skipped

Example: Timer fires at 12:00:00.001 (1ms late)

recomputeNextRuns sets nextRunAtMs to 14:00:00 (next occurrence)
runDueJobs checks: 12:00:00.001 >= 14:00:00.000 → false → 12:00 job skipped

Solution

Reorder operations in onTimer:

Load store WITHOUT recomputing (preserve stored nextRunAtMs)
Check and run due jobs using stored nextRunAtMs values
THEN recompute next runs for subsequent executions
Persist and arm timer

Changes

src/cron/service/store.ts: Add skipRecompute option to ensureLoaded
src/cron/service/timer.ts: Reorder operations, import recomputeNextRuns

Greptile Overview

Greptile Summary

This PR addresses a cron scheduling race by changing the onTimer flow to load the store without recomputing nextRunAtMs, run any due jobs using the persisted nextRunAtMs, then recompute/persist and re-arm the timer.

It also relaxes model provider config validation by making models.providers.*.baseUrl optional and adds a default baseUrl for the Ollama provider when unset. Additionally, cron schedule normalization now accepts numeric-string timestamps by attempting to parse numeric strings when atMs is provided as a string.

Confidence Score: 3/5

This PR is close, but there are a couple of behavior-changing scheduling/parsing issues to resolve before merging.
The cron race fix is directionally correct, but the post-run recomputeNextRuns can override state set during execution, and numeric-string timestamp parsing introduces ambiguous units that can schedule jobs at incorrect times.
src/cron/service/timer.ts, src/cron/normalize.ts

Context used:

Context from dashboard - CLAUDE.md (source)
Context from dashboard - AGENTS.md (source)

When configuring Ollama via CLI (e.g., 'openclaw config set models.providers.ollama.apiKey'), the validation was failing because baseUrl was required. Changes: - Make baseUrl optional in ModelProviderSchema - Apply default baseUrl 'http://localhost:11434' for Ollama in applyModelDefaults Fixes openclaw#9652

When users send atMs as a numeric string (e.g., '1234567890') via the cron tool, the normalization was failing to parse it correctly because parseAbsoluteTimeMs expects ISO date strings. This caused schedule.at to be undefined, which made computeJobNextRunAtMs return undefined, leaving jobs without state.nextRunAtMs set. Jobs would never execute because the scheduler couldn't determine when they were due. Changes: - Add parseNumericStringToMs helper to convert numeric strings to timestamps - Use it as fallback in coerceSchedule when parseAbsoluteTimeMs fails Fixes openclaw#9668

When the timer fires slightly after the scheduled time (even 1ms late), the previous order of operations caused jobs to be skipped: 1. ensureLoaded called recomputeNextRuns, which advanced nextRunAtMs to the NEXT occurrence (e.g., 14:00 instead of 12:00) 2. runDueJobs then checked if jobs were due, but nextRunAtMs was already in the future, so no jobs ran The fix reorders operations in onTimer: 1. Load store WITHOUT recomputing (preserve stored nextRunAtMs) 2. Check and run due jobs using stored nextRunAtMs values 3. THEN recompute next runs for subsequent executions 4. Persist and arm timer This ensures jobs are checked against their original scheduled times before any recomputation happens. Changes: - store.ts: Add skipRecompute option to ensureLoaded - timer.ts: Reorder operations, call recomputeNextRuns after runDueJobs Fixes openclaw#9661

greptile-apps

_{2 files reviewed, 2 comments}

_{Edit Code Review Agent Settings | Greptile}

greptile-apps · 2026-02-05T15:28:14Z

src/cron/service/timer.ts

+      // Load without recomputing to preserve stored nextRunAtMs values
+      await ensureLoaded(state, { forceReload: true, skipRecompute: true });
+      // Check and run due jobs using stored nextRunAtMs
      await runDueJobs(state);
+      // Then recompute next runs for subsequent executions
+      recomputeNextRuns(state);
      await persist(state);


Next run overwritten

executeJob already sets job.state.nextRunAtMs based on endedAt (and may disable one-shot jobs). Calling recomputeNextRuns(state) immediately after runDueJobs recomputes nextRunAtMs for all enabled jobs based on a fresh nowMs(), which can overwrite the post-run state for jobs that just executed. This will produce incorrect scheduling when a job run crosses a schedule boundary or when endedAt differs materially from the recompute timestamp.

Also appears in src/cron/service/store.ts where recomputeNextRuns is called on load (intended there), so the safer fix here is to avoid recomputing for jobs that just ran (or only recompute before deciding what’s due).

Prompt To Fix With AI

This is a comment left during a code review. Path: src/cron/service/timer.ts Line: 41:47 Comment: **Next run overwritten** `executeJob` already sets `job.state.nextRunAtMs` based on `endedAt` (and may disable one-shot jobs). Calling `recomputeNextRuns(state)` immediately after `runDueJobs` recomputes `nextRunAtMs` for *all* enabled jobs based on a fresh `nowMs()`, which can overwrite the post-run state for jobs that just executed. This will produce incorrect scheduling when a job run crosses a schedule boundary or when `endedAt` differs materially from the recompute timestamp. Also appears in `src/cron/service/store.ts` where `recomputeNextRuns` is called on load (intended there), so the safer fix here is to avoid recomputing for jobs that just ran (or only recompute before deciding what’s due). How can I resolve this? If you propose a fix, please make it concise.

greptile-apps · 2026-02-05T15:28:15Z

src/cron/normalize.ts

+function parseNumericStringToMs(value: string): number | null {
+  const trimmed = value.trim();
+  if (!trimmed) return null;
+  // Handle numeric strings (e.g., "1234567890") by parsing as timestamp
+  const numeric = Number(trimmed);
+  if (Number.isFinite(numeric) && numeric > 0) {
+    return numeric;
+  }
+  return null;


Numeric timestamp ambiguity

parseNumericStringToMs treats any positive numeric string as a millisecond timestamp. This will silently accept common epoch-seconds values (10 digits) and interpret them as milliseconds, shifting scheduled times by ~1000×. Previously these strings would fall through unless parseAbsoluteTimeMs accepted them, so this is a behavior change that can schedule jobs at unintended times.

If numeric strings are supported, this needs a clear unit rule (e.g., detect seconds vs ms by magnitude) or explicitly document/validate the expected format.

Prompt To Fix With AI

This is a comment left during a code review. Path: src/cron/normalize.ts Line: 20:28 Comment: **Numeric timestamp ambiguity** `parseNumericStringToMs` treats any positive numeric string as a millisecond timestamp. This will silently accept common epoch-*seconds* values (10 digits) and interpret them as milliseconds, shifting scheduled times by ~1000×. Previously these strings would fall through unless `parseAbsoluteTimeMs` accepted them, so this is a behavior change that can schedule jobs at unintended times. If numeric strings are supported, this needs a clear unit rule (e.g., detect seconds vs ms by magnitude) or explicitly document/validate the expected format. How can I resolve this? If you propose a fix, please make it concise.

Root cause: When timer fires even 1ms late, ensureLoaded() calls recomputeNextRuns() which advances nextRunAtMs to the NEXT occurrence. Then runDueJobs() checks now >= nextRunAtMs → false → job skipped. Solution (based on openclaw#9661 analysis and PR openclaw#9684): 1. Add skipRecompute option to ensureLoaded() 2. In onTimer: load with skipRecompute=true → run due jobs → THEN recompute This preserves runtime reload functionality (Gemini review feedback) while fixing the timer race condition. Fixes: openclaw#9542 Related: openclaw#9661, openclaw#9684

christianhaberl · 2026-02-05T19:03:50Z

I've submitted #9833 with a focused version of the core cron fix (only store.ts + timer.ts changes).

The skipRecompute approach in your PR is correct and I've confirmed it works — tested on Raspberry Pi 5 with 11+ cron runs over 22 minutes, no skips.

However, this PR includes unrelated changes (Ollama defaults, schema relaxation, numeric string parsing) that make it harder to review and increase regression risk. #9833 is scoped to just the cron fix for easier merging.

Either PR would fix the issue — just offering a minimal alternative.

openclaw-barnacle · 2026-02-21T04:36:03Z

This pull request has been automatically marked as stale due to inactivity.
Please add updates or it will be closed.

steipete · 2026-02-24T05:46:31Z

Closing as AI-assisted stale-fix triage.

Linked issue #9661 ("[Bug]: Cron jobs skipped due to race condition - recomputeNextRuns called before runDueJobs") is currently closed and was closed on 2026-02-07T02:06:09Z with state reason completed.
Given that issue is closed, this fix PR is no longer needed in the active queue and is being closed as stale.

If this specific implementation is still needed on current main, please reopen #9684 (or open a new focused fix PR) and reference #9661 for fast re-triage.

Shadow added 3 commits February 5, 2026 15:52

greptile-apps bot reviewed Feb 5, 2026

View reviewed changes

divol89 mentioned this pull request Feb 5, 2026

[Bug] Cron jobs never execute due to recomputeNextRuns resetting nextRunAtMs on every timer tick (related to #9661) #9725

Closed

This was referenced Feb 5, 2026

fix: prevent cron jobs from being skipped when timer fires christianhaberl/openclaw#1

Open

fix: prevent cron jobs from being skipped when timer fires #9833

Closed

HenryLoenwind mentioned this pull request Feb 5, 2026

fix(cron): prevent recomputeNextRuns from skipping due jobs in onTimer #9823

Merged

4 tasks

thewilloftheshadow force-pushed the main branch from bfc1ccb to f92900f Compare February 15, 2026 18:46

openclaw-barnacle bot added stale Marked as stale due to inactivity and removed stale Marked as stale due to inactivity labels Feb 21, 2026

steipete closed this Feb 24, 2026

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

fix: cron race condition - run due jobs before recomputing nextRunAtMs (#9661)#9684

fix: cron race condition - run due jobs before recomputing nextRunAtMs (#9661)#9684
divol89 wants to merge 3 commits intoopenclaw:mainfrom
divol89:fix/9661-cron-race-condition

divol89 commented Feb 5, 2026 •

edited by greptile-apps bot

Loading

Uh oh!

greptile-apps bot left a comment

Uh oh!

greptile-apps bot Feb 5, 2026

Uh oh!

greptile-apps bot Feb 5, 2026

Uh oh!

christianhaberl commented Feb 5, 2026

Uh oh!

openclaw-barnacle bot commented Feb 21, 2026

Uh oh!

steipete commented Feb 24, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

Uh oh!

Conversation

divol89 commented Feb 5, 2026 • edited by greptile-apps bot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Problem

Solution

Changes

Related

Greptile Overview

Greptile Summary

Confidence Score: 3/5

Uh oh!

greptile-apps bot left a comment

Choose a reason for hiding this comment

Uh oh!

greptile-apps bot Feb 5, 2026

Choose a reason for hiding this comment

Uh oh!

greptile-apps bot Feb 5, 2026

Choose a reason for hiding this comment

Uh oh!

christianhaberl commented Feb 5, 2026

Uh oh!

openclaw-barnacle bot commented Feb 21, 2026

Uh oh!

steipete commented Feb 24, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

divol89 commented Feb 5, 2026 •

edited by greptile-apps bot

Loading