Skip to content

fix(cron): don't skip first-run cron jobs past grace window - #41307

Closed
cmcejas wants to merge 1 commit into
NousResearch:mainfrom
cmcejas:fix/cron-jobs-never-execute
Closed

fix(cron): don't skip first-run cron jobs past grace window#41307
cmcejas wants to merge 1 commit into
NousResearch:mainfrom
cmcejas:fix/cron-jobs-never-execute

Conversation

@cmcejas

@cmcejas cmcejas commented Jun 7, 2026

Copy link
Copy Markdown
Contributor

Problem

Cron jobs with last_run_at=None (never run) were being silently skipped by _get_due_jobs_locked() when the computed next_run_dt fell outside the grace window. The grace-window fast-forward logic is correct for jobs that have already run, but wrong for first-run jobs the user just created or manually triggered.

Root Cause

In _get_due_jobs_locked(), when a job's next_run_dt is past the grace window, the code fast-forwards to the next future occurrence. For cron jobs that have never run (last_run_at is None/empty), this means the first run is silently skipped.

Fix

When kind=='cron' and last_run_at is falsy, bypass the grace-window check and let the job run. The grace-window logic still applies to cron jobs that have already run at least once.

Fixes #41037

Cron jobs with last_run_at=None (never run) were being silently skipped
by _get_due_jobs_locked() when the computed next_run_dt fell outside the
grace window. The grace-window fast-forward logic is correct for jobs
that have already run, but wrong for first-run jobs the user just created
or manually triggered.

Fix: when kind=='cron' and last_run_at is falsy, bypass the grace-window
check and let the job run.

Fixes NousResearch#41037
@cmcejas

cmcejas commented Jun 7, 2026

Copy link
Copy Markdown
Contributor Author

Note: this PR is complementary to the other open PRs for #41037 (#41130 by @kyssta-exe and #41269 by @rodboev). Those PRs address the immediate dispatch problem — making action=run actually execute the job right away instead of just setting next_run_at and waiting for the next tick.

This PR addresses a different root cause: even when trigger_job() correctly sets next_run_at = now(), the scheduler's periodic tick can still silently skip first-run cron jobs. The grace-window fast-forward logic in _get_due_jobs_locked() treats a never-run job (last_run_at = null) the same as a job that has already run, and if the computed next_run_at falls outside the grace window, the job gets fast-forwarded past its first execution.

Both fixes are needed for a complete solution:

The fixes are independent and can merge in any order.

@daimon-nous daimon-nous Bot added type/bug Something isn't working comp/cron Cron scheduler and job management P1 High — major feature broken, no workaround labels Jun 7, 2026
@liuhao1024

Copy link
Copy Markdown
Contributor

✅ Verification — cron first-run grace window fix

Reviewed cron/jobs.py for the first-run cron job skip prevention in _get_due_jobs_locked().

  • Root cause confirmed: The grace-window fast-forward logic treats last_run_at=null (never run) the same as a job that already ran. When a user manually triggers a cron job via action=run, the scheduler sets next_run_at = now(), but on the next tick if the grace window check fires first, the job gets fast-forwarded past its first execution — silently skipping it.
  • Fix correctness: Adding if kind == "cron" and not last_run: before the fast-forward block ensures never-run cron jobs are always treated as due, regardless of grace window calculations. The continue correctly moves to the next candidate without computing a new next_run.
  • Scope safety: The guard is limited to kind == "cron" only — interval jobs are unaffected. This is correct because interval jobs have different first-run semantics.
  • Edge case: not last_run handles both None and empty string "", which are the two representations of "never run" in the database layer.

Clean fix — no issues found.

@teknium1 teknium1 added sweeper:risk-session-state Sweeper risk: may lose/corrupt/mis-associate session or context state sweeper:risk-message-delivery Sweeper risk: may drop, duplicate, misroute, or suppress messages sweeper:risk-compatibility Sweeper risk: may break existing users, config, migrations, defaults, or upgrades sweeper:blast-moderate Sweeper blast radius: moderate — a subsystem or single platform labels Jun 21, 2026
@teknium1

Copy link
Copy Markdown
Contributor

Automated hermes-sweeper review: this cron first-run skip fix is now implemented on main by the later merged catch-up behavior.

Evidence:

  • cron/jobs.py:1516 now handles stale recurring jobs past the grace window by fast-forwarding next_run_at but continuing execution once now.
  • cron/jobs.py:1544 documents the key behavior: "Fall through to due.append(job) — execute once now".
  • tests/cron/test_jobs.py:739 covers the new contract: a stale recurring job past grace is returned as due and fast-forwards its next run.
  • The broader fix landed through fix(cron): run missed-grace jobs once instead of deferring forever #50062, commit 6777a6bd67ccabd92455845736b17150a96c6a14, which replaced the old skip/continue behavior with run-once catch-up.

Thanks to @cmcejas for identifying the never-run cron edge case and to the commenters who linked the related #41037 fixes; the current main behavior now covers this PR's requested guarantee.

@teknium1 teknium1 closed this Jun 29, 2026
@teknium1 teknium1 added the sweeper:implemented-on-main Sweeper: behavior already present on current main label Jun 29, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

comp/cron Cron scheduler and job management P1 High — major feature broken, no workaround sweeper:blast-moderate Sweeper blast radius: moderate — a subsystem or single platform sweeper:implemented-on-main Sweeper: behavior already present on current main sweeper:risk-compatibility Sweeper risk: may break existing users, config, migrations, defaults, or upgrades sweeper:risk-message-delivery Sweeper risk: may drop, duplicate, misroute, or suppress messages sweeper:risk-session-state Sweeper risk: may lose/corrupt/mis-associate session or context state type/bug Something isn't working

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Cron jobs never execute: last_run_at always null after manual trigger

3 participants