Skip to content

fix(cron): prevent long-running scheduled scripts from running twice - #63406

Closed
morluto wants to merge 1 commit into
NousResearch:mainfrom
morluto:fix/cron-prevent-duplicate-scripts
Closed

fix(cron): prevent long-running scheduled scripts from running twice#63406
morluto wants to merge 1 commit into
NousResearch:mainfrom
morluto:fix/cron-prevent-duplicate-scripts

Conversation

@morluto

@morluto morluto commented Jul 12, 2026

Copy link
Copy Markdown
Contributor

What does this PR do?

Prevents long-running one-shot scheduled scripts from being started a second time while their original execution is still active.

One-shot jobs use a time-limited run claim to prevent duplicate execution. Existing code refreshes that claim while the agent is running, but not while either blocking script path is running:

  • script-only jobs using no_agent
  • scripts that run before agent or wake-gate evaluation

Scripts can run longer than the claim lifetime. Once the claim appears stale, another scheduler scan can claim and start the same job again.

This PR keeps the existing owner-bound claim alive while either script path is running. The heartbeat:

  • only runs for claimed one-shot jobs
  • uses the claim owner captured before the script starts
  • preserves the active profile's cron-store context in the background thread
  • stops when script execution finishes
  • does not refresh a claim that has been replaced by another owner

Recurring, unclaimed, and quick jobs are unchanged. Script timeout, output, delivery, and wake-gate behavior are also unchanged.

Related Issue

Follow-up to #62002, #62014, and #62155.

No new issue was opened because this is a missed sibling path of the existing run-claim heartbeat fix. Searches of open and merged PRs found no submitted fix covering the blocking script paths.

Type of Change

  • 🐛 Bug fix
  • ✨ New feature
  • 💥 Breaking change
  • 📚 Documentation
  • 🎨 Style / UI
  • ♻️ Refactor
  • ⚡ Performance
  • ✅ Tests
  • 🔧 Chore

Changes Made

  • Added a claim-heartbeat wrapper around blocking scheduled-script execution.
  • Applied it to both script-only and pre-agent script paths.
  • Reused the scheduler's existing 60-second claim-heartbeat cadence.
  • Propagated the active profile context into the heartbeat thread.
  • Added regression coverage proving:
    • a second scheduler scan cannot redispatch a script after the original claim lifetime
    • profile-specific cron stores remain isolated
    • a replacement claim owner is not refreshed

How to Test

uv pip install -e '.[all,dev]'
pytest tests/cron/test_script_claim_heartbeat.py tests/cron/test_scheduler.py tests/cron/test_jobs.py -q

Result:

348 passed, 2 warnings in 65.40s

The warnings are existing unawaited-coroutine warnings outside this change.

Also validated:

  • Full cron suite: 682 passed
  • Autoreview: no actionable findings
  • Ruff and git diff --check: clean

Tested on Linux with Python 3.12.3.

Checklist

Code

  • I've read the contributing guide
  • My commits follow Conventional Commits
  • I've searched open and merged PRs for an existing fix
  • This PR contains one focused logical change
  • I've installed the declared development and test dependencies
  • I've run the tests covering the affected scheduler and claim lifecycle; all 348 pass
  • I've added regression tests for the changed behavior
  • I've tested the affected scheduler behavior on Linux
  • I've considered cross-platform behavior; the change uses only standard-library threading and context propagation

Documentation & Housekeeping

  • Documentation changes are not needed because no user-facing interface or configuration changed
  • No new environment variables or configuration keys were added
  • No tool schemas or core tool surface changed
  • No AGENTS.md update is needed

Screenshots / Logs

Not applicable. This change affects background cron coordination and is covered by the regression tests above.

Reviewer Guidance

Suggested review order:

  1. The script heartbeat helper in cron/scheduler.py
  2. Its two script-execution call sites
  3. tests/cron/test_script_claim_heartbeat.py

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

Copy link
Copy Markdown
Collaborator

Merged via #64365. Your commit cherry-picked with authorship preserved (rebase-merge). The test coverage (heartbeat-keeps-job-alive, captured-claim-owner, skips-recurring) is excellent — especially the threading.Event-based barriers for deterministic race-window reproduction.

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.

3 participants