Repository navigation
Fail closed when /schedule time math overflows or mktime fails - #358
Merged
Merged
Conversation
every hour used raw now+3600 (unlike every N hours / add_seconds), so a near-int64 last-fire could wrap to a negative next_fire_at. mktime failure returned -1, which list_due treats as always due and a recurring recompute would tight-loop. Return 0 / a parse error instead. Co-authored-by: Tyler Reckart <tylerreckart@users.noreply.github.com>
This was referenced Sep 19, 2026
…h-fail-closed # Conflicts: # CHANGELOG.md
tylerreckart
marked this pull request as ready for review
September 21, 2026 12:58
tylerreckart
enabled auto-merge (squash)
September 21, 2026 12:58
…h-fail-closed # Conflicts: # CHANGELOG.md
…h-fail-closed # Conflicts: # CHANGELOG.md
…h-fail-closed # Conflicts: # CHANGELOG.md
…h-fail-closed # Conflicts: # CHANGELOG.md # tests/test_schedule_parser.cpp
…h-fail-closed # Conflicts: # CHANGELOG.md
tylerreckart
added a commit
that referenced
this pull request
Sep 21, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Problem
Two schedule-time helpers could persist
next_fire_atvalues thatlist_duetreats as immediately due (next_fire_at <= now):every hour/hourlyused rawnow + 3600(andafter + 3600on recompute).#318already overflow-checksin N hoursandevery N hoursviaadd_seconds; the hourly keyword was missed. A last-fire nearINT64_MAXwraps to a negative next fire.mktimefailure returned(time_t)-1frommake_local_epoch.-1is always<= now, so a recurringnext_fire_for_recurrecompute would tight-loop LLM runs. Distinct from Reject impossible /schedule calendar dates instead of overflowing the month #349 (impossible calendar dates at parse time).Recovery (
finalize_orphaned_scheduled_task_leases) and the scheduler tick both callnext_fire_for_recuron hourly rows.Fix
next_fire_for_recuruseadd_seconds(..., 1, 3600)and fail closed (interval too large/ return0).make_local_epochreturns0onmktime == (time_t)-1(same sentinelnext_fire_for_recuralready documents as "cannot compute").next_local_at/next_local_weekday_atand theat/tomorrow/on/every day/every <weekday>parse paths reject<= 0instead of storing it.Ordinary
every hourfrom a wall-clocknowis unchanged (now + 3600).Tests
unit_schedule_parser:next_fire_for_recur({"every":"hour"}, INT64_MAX-100)returns0.parse_schedule_phrase("every hour", INT64_MAX-100)is not ok.on 0001-01-01does not persistnext_fire_at = -1.Suite 8/8 locally (75 assertions) + ASan + UBSan.
Independently mergeable against
main(0456350).git merge-tree --write-treevs #334 and #349 is CLEAN (different hunks: resume helpers /parse_ymdcalendar check). CHANGELOG[Unreleased]sibling-conflicts after the first of #321–#357 merges, same as the rest of the series.Note
Medium Risk
Scheduler recurring-task recompute is on the hot path; incorrect sentinels could tight-loop runs, though behavior for normal wall-clock schedules is unchanged. Unresolved test-file conflict markers in the PR would break the build until resolved.
Overview
Makes
/scheduletime math fail closed so bad or overflowing timestamps are never stored asnext_fire_atvalues that the scheduler treats as immediately due.Hourly recurrence (
every hour/hourlyand{"every":"hour"}recompute) now uses the same overflow-checkedadd_secondspath asevery N hours, returning parse errors or0fromnext_fire_for_recurinstead of wrapping nearINT64_MAX.Local calendar math treats
mktimefailure as unusable:make_local_epochreturns0instead of-1, andnext_local_at/next_local_weekday_atplusat,tomorrow,on, daily, and weekly parse paths reject non-positive fire times with a shared error instead of persisting them.Tests add overflow and extreme-
nowcases; the diff still contains unresolved merge conflict markers intests/test_schedule_parser.cppalongside calendar-date tests frommainthat need to be merged before land.Reviewed by Cursor Bugbot for commit af1a19a. Bugbot is set up for automated code reviews on this repo. Configure here.