fix(cron): stop one-shot At jobs from re-firing every poll - #5039
Conversation
📝 WalkthroughWalkthroughAt-scheduled cron jobs are now terminated after one execution even when automatic deletion is disabled. Retained jobs record their result, become disabled, and are excluded from later due-job selection. A regression test covers this behavior. ChangesCron At job lifecycle
Suggested labels: Suggested reviewers: Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
A Schedule::At job created via the RPC handler defaults delete_after_run to false (schemas.rs handle_add), and add_shell_job never sets it at all, so such jobs are not is_one_shot_auto_delete. persist_job_result then routes them through reschedule_after_run, which writes next_run = next_run_for_schedule(At, now) = the fixed (now past) instant. due_jobs selects enabled=1 AND next_run<=now, so the past 'at' stays perpetually due and the job re-executes on every poll interval forever — burning model cost and repeating external effects for agent jobs. Fix at the scheduler: a fixed-instant (At) job is inherently one-shot, so terminate it after a single run regardless of delete_after_run. An auto-delete job that succeeded is still removed; every other At job is kept disabled so its run history stays inspectable. This covers both the agent and shell RPC entry points; the narrower schemas.rs default-mirror would miss add_shell_job. Test: persist_job_result_disables_at_job_without_delete_flag asserts an At job with delete_after_run=false is disabled after one run and no longer returned by due_jobs.
90c1a16 to
8c83b9e
Compare
|
| Filename | Overview |
|---|---|
| src/openhuman/cron/scheduler.rs | Core fix: persist_job_result now gates on Schedule::At before is_one_shot_auto_delete, correctly terminating all fixed-instant jobs after one run. If remove_job fails for an auto-delete-success At job, the job is neither removed nor disabled and can still re-fire — a minor gap worth a fallback disable. |
| src/openhuman/cron/scheduler_tests.rs | New regression test covers the previously-buggy delete_after_run=false At path; checks both the enabled state and due_jobs exclusion after a single run. Existing one-shot tests continue to exercise the auto-delete branches through the refactored At block. |
Flowchart
%%{init: {'theme': 'neutral'}}%%
flowchart TD
A[persist_job_result called] --> B{Schedule::At?}
B -- No --> C[reschedule_after_run\nnext_run = next occurrence]
B -- Yes --> D{is_one_shot_auto_delete\nAND success?}
D -- Yes --> E[remove_job\ndelete row]
D -- No --> F[record_last_run\nsuccess flag passed]
F --> G[update_job\nenabled = false]
E -- remove_job fails --> H[warn log\njob stays enabled\npotential re-fire]
G -- update_job fails --> I[warn log\njob stays enabled\npotential re-fire]
E --> J[return success]
F --> J
C --> J
Reviews (1): Last reviewed commit: "fix(cron): stop one-shot At jobs from re..." | Re-trigger Greptile
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
| if is_one_shot_auto_delete(job) && success { | ||
| if let Err(e) = remove_job(config, &job.id) { | ||
| tracing::warn!("Failed to remove one-shot cron job after success: {e}"); | ||
| } | ||
| } else { |
There was a problem hiding this comment.
remove_job fails, the At job is neither removed nor disabled — its next_run is still the past instant, so it remains perpetually due and will re-fire on the next poll, exactly the bug this PR is fixing. Adding a fallback update_job(enabled: false) after the warn closes this gap.
| if is_one_shot_auto_delete(job) && success { | |
| if let Err(e) = remove_job(config, &job.id) { | |
| tracing::warn!("Failed to remove one-shot cron job after success: {e}"); | |
| } | |
| } else { | |
| if is_one_shot_auto_delete(job) && success { | |
| if let Err(e) = remove_job(config, &job.id) { | |
| tracing::warn!("Failed to remove one-shot cron job after success: {e}"); | |
| // Fallback: disable so the job does not re-fire on the next poll. | |
| let _ = update_job( | |
| config, | |
| &job.id, | |
| CronJobPatch { | |
| enabled: Some(false), | |
| ..CronJobPatch::default() | |
| }, | |
| ); | |
| } | |
| } else { |
2 participants
Summary
Schedule::Atcron jobs re-fire on every poll interval forever whendelete_after_runis not set.Atjob as inherently one-shot in the scheduler, so it terminates after a single run regardless of the flag or entry point (agent and shell RPC).Atjob withdelete_after_run=falseis disabled after one run and is never re-selected bydue_jobs.Problem
Schedule::Atis a fixed instant. When such a job runs,persist_job_resultdecides whether to finish it (one-shot) or reschedule it:is_one_shot_auto_delete(job)isjob.delete_after_run && matches!(schedule, At).delete_after_runtomatches!(schedule, At{..})(tools/add.rs), so tool-createdAtjobs are correctly one-shot.false(schemas.rshandle_add), andadd_shell_jobnever sets it at all (column default0).So an
Atjob created over RPC is notis_one_shot_auto_delete, and falls through toreschedule_after_run, which writesnext_run = next_run_for_schedule(&At, now)= the (now past) instant.due_jobsselectsenabled = 1 AND next_run <= now, so the pastatstays perpetually due and the job re-executes on every poll — burning model cost and repeating external side effects for agent jobs.Solution
Fix at the scheduler rather than mirroring the RPC default (a default-mirror would miss
add_shell_job). A fixed-instant job can never have a legitimate future occurrence, so terminate everyAtjob after a single run:Atjob → kept disabled (enabled = false) with its last run recorded, so history stays inspectable and it never re-fires.Atschedules are unaffected — they stillreschedule_after_run.Inside the new
Atbranch,is_one_shot_auto_delete(job)reduces tojob.delete_after_run, so the existing helper and its unit tests remain valid.Submission Checklist
persist_job_result_disables_at_job_without_delete_flagcovers the previously-buggydelete_after_run=falsepath; existing..._success_deletes_one_shot/..._failure_disables_one_shotcover the auto-delete arms, which now route through the sameAtbranch.Atbranch:remove_job,record_last_run+ disable, and the guard) are all exercised by the three one-shot tests.cargo test -p openhuman --lib cron::schedulerpasses (one unrelated pre-existing failure noted under Impact).N/A: behaviour-only bug fix, no new/renamed feature.## Related—N/A: no matrix feature touched.N/A: does not touch a release-cut surface.Closes #NNN— no existing issue; this was found by inspection.Impact
Atjobs created viacron.addRPC (agent and shell). No migration; existing rescheduled-to-pastAtrows stop re-firing the next time they run (they get disabled) — or can be deleted normally.cargo test -p openhuman --lib cron::schedulershows one pre-existing, unrelated failure —run_agent_job_returns_error_without_provider_keyoverflows the debug thread stack (deep agent async state machine); it reproduces identically on cleanmainwith this change stashed, and does not touch the scheduler reschedule path.Related
AI Authored PR Metadata
Linear Issue
Commit & Branch
fix/cron-at-oneshot-refire90c1a16e8Validation Run
pnpm --filter openhuman-app format:check— N/A (no frontend change)pnpm typecheck— N/A (no frontend change)cargo test -p openhuman --lib cron::scheduler(new + one-shot family pass)cargo fmtBehavior Changes
Atjobs no longer reschedule after running; they terminate (remove-on-success for auto-delete, else disable).Parity Contract
delete_after_run=trueAtjobs; all non-Atschedules reschedule exactly as before.is_one_shot_auto_deletesemantics unchanged; the new branch is gated strictly onmatches!(schedule, At{..}).