While reading src/lib/cron.ts I noticed describeCron destructures the five cron fields but skips the month:
const [minute, hour, dayOfMonth, , dayOfWeek] = f;
So a schedule that is restricted to certain months gets described by its finer fields alone. runsPerMonth handles the month correctly (it divides by months.size / MONTHS_PER_YEAR), which puts the two halves of the advisory in direct conflict.
Reproduction
$ testsprite schedule create --cron "0 3 * 6 *" ...
This schedule will run ~3 time(s)/month (daily at 03:00). Each run is a full test run.
0 3 * 6 * runs daily at 03:00 during June only. "~3 time(s)/month" and "daily at 03:00" cannot both be true, and they are printed in the same sentence. This is the user-facing path — formatScheduleFrequencyAdvisory is called from src/commands/schedule.ts:245 (create) and :347 (update).
Confirmed against main (40d18d2):
| cron |
actual meaning |
describeCron |
0 3 * 6 * |
daily in June |
daily at 03:00 |
0 3 * 1,7 * |
daily in Jan & Jul |
daily at 03:00 |
0 3 * */6 * |
daily in Jan & Jul |
daily at 03:00 |
30 9 1 3 * |
1 March, yearly |
monthly on day 1 at 09:30 |
The existing tests miss it because describeCron is never exercised with a non-wildcard month, and the one advisory test that uses one (0 3 1 1 *) asserts only the frequency half, not the description.
Suggested fix
None of the phrasings describeCron can produce is able to carry "only in these months", so a restricted month should fall back to the raw expression — the same thing the function already does for every other shape it cannot state exactly, and what its docstring asks for: "a wrong description is worse than the raw expression".
I have the fix and regression tests ready (verified failing on main and passing with the change) and would like to pick this up — /assign.
While reading
src/lib/cron.tsI noticeddescribeCrondestructures the five cron fields but skips the month:So a schedule that is restricted to certain months gets described by its finer fields alone.
runsPerMonthhandles the month correctly (it divides bymonths.size / MONTHS_PER_YEAR), which puts the two halves of the advisory in direct conflict.Reproduction
0 3 * 6 *runs daily at 03:00 during June only. "~3 time(s)/month" and "daily at 03:00" cannot both be true, and they are printed in the same sentence. This is the user-facing path —formatScheduleFrequencyAdvisoryis called fromsrc/commands/schedule.ts:245(create) and:347(update).Confirmed against
main(40d18d2):describeCron0 3 * 6 *daily at 03:000 3 * 1,7 *daily at 03:000 3 * */6 *daily at 03:0030 9 1 3 *monthly on day 1 at 09:30The existing tests miss it because
describeCronis never exercised with a non-wildcard month, and the one advisory test that uses one (0 3 1 1 *) asserts only the frequency half, not the description.Suggested fix
None of the phrasings
describeCroncan produce is able to carry "only in these months", so a restricted month should fall back to the raw expression — the same thing the function already does for every other shape it cannot state exactly, and what its docstring asks for: "a wrong description is worse than the raw expression".I have the fix and regression tests ready (verified failing on
mainand passing with the change) and would like to pick this up —/assign.