Skip to content

fix(cron): guard stale-entry removal with liveness check - #62013

Closed
liuhao1024 wants to merge 1 commit into
NousResearch:mainfrom
liuhao1024:liuhao/cron-bugfix-62002-stale-entry-liveness
Closed

fix(cron): guard stale-entry removal with liveness check#62013
liuhao1024 wants to merge 1 commit into
NousResearch:mainfrom
liuhao1024:liuhao/cron-bugfix-62002-stale-entry-liveness

Conversation

@liuhao1024

Copy link
Copy Markdown
Contributor

What does this PR do?

Fixes a race condition in cron one-shot job stale-entry recovery where get_due_jobs() deletes a job while its run is still alive.

The one-shot dispatch-limit guard (#38758) removes stale entries when completed >= times and the run_claim TTL is expired. However, it only checked the claim age as a liveness signal. A one-shot whose run legitimately outlives the TTL (default 1800s) — for example, stalled on network I/O or a laptop that slept mid-run — satisfies the same condition while the run is still executing.

This caused the job record to be deleted mid-flight:

  • cronjob(action='list') shows the job as gone → looks like it never ran
  • When the run finally completes, mark_job_run() finds nothing → last_run_at / last_status / last_delivery_error are lost
  • If the run failed or delivery failed, there's zero durable trace

The fix adds a same-process liveness check before removal: consult cron.scheduler.get_running_job_ids() to see if the job is still executing in this process. If it is, skip removal regardless of claim age. This covers the common single-gateway case where the ticker and run share the same process.

Related Issue

Fixes #62002

Type of Change

  • 🐛 Bug fix (non-breaking change that fixes an issue)

Changes Made

  • cron/jobs.py: In get_due_jobs(), before removing a stale one-shot entry, check if the job is still executing in this process via cron.scheduler.get_running_job_ids(). If it is, skip removal and log that the run is still alive.

How to Test

  1. Create a one-shot job with repeat=1 and a prompt that stalls >1800s (e.g., blackhole the provider endpoint after the first tool call).
  2. Let the ticker fire the job; while the run is stalled, wait for run_claim age > TTL.
  3. Observe that get_due_jobs() logs "one-shot dispatch limit reached but still executing in this process — skipping removal" and does NOT delete the job.
  4. Let the run complete; mark_job_run() should find the job and update last_run_at / last_status / last_delivery_error correctly.
  5. Verify that cronjob(action='list') shows the job with correct completion status.

Regression test: Existing cron tests pass (416 tests in tests/cron/test_jobs.py).

Checklist

Code

  • I've read the Contributing Guide
  • My commit messages follow Conventional Commits (fix(scope):, feat(scope):, etc.)
  • I searched for existing PRs to make sure this isn't a duplicate
  • My PR contains only changes related to this fix/feature (no unrelated commits)
  • I've run pytest tests/cron/ -q and all tests pass (416 passed, 1 pre-existing flaky test unrelated to this change)
  • I've added tests for my changes (liveness check path is exercised by existing test_jobs.py tests; same-process check guards deletion)
  • I've tested on my platform: macOS 26.5.1 (Apple Silicon)

Documentation & Housekeeping

  • I've updated relevant documentation (README, docs/, docstrings) — or N/A
  • I've updated cli-config.yaml.example if I added/changed config keys — or N/A
  • I've updated CONTRIBUTING.md or AGENTS.md if I changed architecture or workflows — or N/A
  • I've considered cross-platform impact (Windows, macOS) per the compatibility guide — or N/A (liveness check uses thread-safe frozenset, works cross-platform)
  • I've updated tool descriptions/schemas if I changed tool behavior — or N/A

@alt-glitch alt-glitch added type/bug Something isn't working P1 High — major feature broken, no workaround comp/cron Cron scheduler and job management 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 labels Jul 10, 2026
@teknium1

Copy link
Copy Markdown
Contributor

Thanks for submitting the first focused fix for #62002. Reviewed against current origin/main: the same-process running-set guard is correct, but #62014 includes that guard plus the cross-process claim heartbeat needed to keep long live runs safe when another scheduler process scans the store. We are proceeding with the broader implementation in #62014 and crediting this PR as the earliest focused submission. Closing as superseded.

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: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: get_due_jobs stale-entry recovery deletes a live one-shot mid-run when the run outlives its run_claim TTL (run outcome + delivery record lost)

3 participants