fix(cron): avoid consuming skipped manual triggers

[pjbeyer]

What does this PR do?

Fixes a cron scheduler edge case where a due/manual-triggered job can be silently consumed when the in-process running guard already contains that job ID.

Previously, tick() advanced next_run_at for every due job before checking _running_job_ids. If the guard then skipped a job as already running, the job did not execute, last_run_at did not change, but next_run_at had already moved forward. That made manual cron run triggers appear accepted while the job was skipped until a later schedule.

This PR moves advance_next_run(job_id) inside _submit_with_guard() after the running guard accepts the job and before the worker starts. Skipped jobs remain due, so manual triggers are not consumed without execution or visible skipped/already-running status. Accepted jobs keep the existing at-most-once behavior, and mark_job_run() still overwrites next_run_at on completion.

Related Issue

No public issue number.

Related/overlapping upstream work:

• Related: fix(cron): execute job immediately on action=run instead of waiting for next tick (#41037) #41269 (cronjob(action="run") immediate execution and skipped-run visibility)

Type of Change

• 🐛 Bug fix (non-breaking change that fixes an issue)
• ✨ New feature (non-breaking change that adds functionality)
• 🔒 Security fix
• 📝 Documentation update
• ✅ Tests (adding or improving test coverage)
• ♻️ Refactor (no behavior change)
• 🎯 New skill (bundled or hub)

Changes Made

• cron/scheduler.py
  • Stop advancing all due jobs before dispatch.
  • Advance next_run_at only after _running_job_ids accepts the job.
  • Release the running guard if advance/submit fails before the worker owns cleanup.
• tests/cron/test_scheduler.py
  • Add regression coverage for an already-running due job to ensure tick() does not call advance_next_run() or run_job() for skipped jobs.

How to Test

1. Run the specific regression test:

   uv run python -m pytest tests/cron/test_scheduler.py::TestRunJobSessionPersistence::test_tick_does_not_advance_skipped_already_running_job -q -o 'addopts='

2. Run the scheduler test file with the project wrapper:

   scripts/run_tests.sh tests/cron/test_scheduler.py

3. Run the cron test package:

   uv run python -m pytest tests/cron -q -o 'addopts='

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/ -q and all tests pass
• I've added tests for my changes (required for bug fixes, strongly encouraged for features)
• I've tested on my platform: macOS 26.5.1

Documentation & Housekeeping

• I've updated relevant documentation (README, docs/, docstrings) — N/A, focused scheduler bug fix covered by code comments + regression test
• I've updated cli-config.yaml.example if I added/changed config keys — N/A
• I've updated CONTRIBUTING.md or AGENTS.md if I changed architecture or workflows — N/A
• I've considered cross-platform impact (Windows, macOS) per the compatibility guide
• I've updated tool descriptions/schemas if I changed tool behavior — N/A

Screenshots / Logs

uv run python scripts/check-windows-footguns.py --diff upstream/main
✓ No Windows footguns found (2 file(s) scanned).

uv run python -m pytest tests/cron/test_scheduler.py::TestRunJobSessionPersistence::test_tick_does_not_advance_skipped_already_running_job -q -o 'addopts='
1 passed in 1.33s

scripts/run_tests.sh tests/cron/test_scheduler.py
135 tests passed, 0 failed

uv run python -m pytest tests/cron -q -o 'addopts='
404 passed in 12.52s

[liuhao1024]

Code Review: Verification

Reviewed by: automated PR review

This PR is clean — no substantive issues found.

What it does: Moves advance_next_run() from the batch loop (before any execution) to inside _submit_with_guard() (after the in-flight dedup guard accepts the job). This ensures that skipped jobs — those already running from a prior tick — don't get their next_run_at advanced, preserving manual trigger semantics.

Assessment:

• The logic is correct: advance_next_run is called only after _running_job_ids.add(job_id) succeeds, so skipped jobs remain "due" and manual triggers are not silently consumed
• The try/except around advance_next_run properly cleans up _running_job_ids on failure, preventing leaked running-set entries
• The existing mark_job_run() overwrite of next_run_at on completion is preserved, maintaining at-most-once semantics for completed runs
• Test specifically validates that a skipped (already-running) job does not advance its next_run_at
• Thread safety is maintained: _running_lock guards both the dedup check and the running-set cleanup

Well-scoped concurrency fix with clear before/after semantics.

[teknium1]

Thanks for the focused regression analysis. This is an automated hermes-sweeper review; current main has since taken a different path that satisfies the manual-trigger guarantee.

• Commit 65d7c7fafdf1719fc71ea35466b5c42a6ab1bf15 (fix(cron): execute job immediately on action='run') changed manual cronjob(action="run") to claim and execute immediately.
• tools/cronjob_tools.py:604 claims then calls run_one_job(), and tools/cronjob_tools.py:838 routes run/run_now/trigger through that path.
• tests/tools/test_cronjob_run_immediate.py:20 covers both immediate execution and visible execution_skipped handling for a lost claim.
• The remaining pre-dispatch advancement in cron/scheduler.py:3602 is currently documented as intentional for already-running scheduled jobs, so this PR would now change that separate policy rather than repair the manual-run behavior.

Closing as implemented on main.