feat(reminders): support CRON_TZ prefix for timezone-aware cron schedules - #1789
feat(reminders): support CRON_TZ prefix for timezone-aware cron schedules#1789nixie-ai wants to merge 5 commits into
Conversation
…ules (netclaw-dev#1788) Cron reminder schedules were always evaluated in UTC because CronScheduleHelper hardcoded TimeZoneInfo.Utc. Add support for the Vixie crontab CRON_TZ=<time-zone-id> prefix: - CronScheduleHelper.SplitTimeZone strips/resolves the optional prefix via TimeZoneInfo.FindSystemTimeZoneById and passes the zone to Cronos GetNextOccurrence (DST-aware). Unknown zones produce a clear CronFormatException instead of a confusing parse error. - TryParse gains an out TimeZoneInfo overload; Describe reports the zone instead of hardcoded 'UTC'. - Stored expressions keep the prefix as-is, so re-scheduling in ReminderManagerActor.ScheduleDefinitionAsync picks up the zone with no proto or schema changes. - set_reminder tool description and ReminderScheduleParser error message document the prefix. Default behavior is unchanged: expressions without the prefix still evaluate in UTC. Fixes netclaw-dev#1788
823034d to
5ce7815
Compare
Aaronontheweb
left a comment
There was a problem hiding this comment.
Thanks. The DST-aware Cronos change looks good.
I have one correctness request and one documentation request:
-
The parser currently splits a timezone ID at the first space. Windows IDs such as
Eastern Standard Timetherefore fail. Please either support quoted IDs or explicitly require IANA identifiers in the parser contract, tool description, and tests. -
Please update
feeds/skills/.system/files/netclaw-operations/references/scheduling.mdwith theCRON_TZguidance. Please also increase thenetclaw-operationsskill version.
We can handle the broader identity integration, confirmation formatting, culture support, and behavioral eval coverage as maintainer follow-up work.
Aaronontheweb
left a comment
There was a problem hiding this comment.
Had Codex leave a comment while I was reviewing with it - but TL;DR; I think as long as you get the timezone culture parsing sorted out along with some guidance to the LLM via the netclaw-operations skill, we should be able to take this and run evals with it to see if small models can schedule correctly using the correct local timezones.
Per netclaw-dev#1789 (review): 1. Make the CRON_TZ zone contract explicit instead of supporting quoted Windows IDs: XML docs on CronScheduleHelper and SplitTimeZone state IANA-only, unknown/empty-zone errors now point at the IANA format (e.g. 'Europe/Brussels') so 'CRON_TZ=Eastern Standard Time' fails with guidance rather than a bare 'Unknown time zone Eastern'. Tool description and ReminderScheduleParser error carry the same rule. 2. Add a 'Cron time zones (CRON_TZ)' section to the netclaw-operations scheduling reference (UTC default, syntax, DST note, IANA-only rule, loose-name translation guidance) and add a prefixed example to the schedule-type table. Bump netclaw-operations 2.40.0 -> 2.41.0. Tests: Windows-style zone rejection (with IANA hint in the message), IANA guidance in unknown-zone errors.
|
Thanks for the review! Addressed both requests in 770de9b: 1. Time zone identifier contract — IANA-only (your second option). I went with an explicit IANA requirement rather than quoted-ID support: IANA ids (
2. Skill docs. Added a Build clean, Reminders test suite 146/146 passing. Happy to run evals once you have them. |
Implements #1788.
Adds support for the Vixie crontab
CRON_TZ=<time-zone-id>prefix on reminder cron schedules, e.g.CRON_TZ=Europe/Brussels 0 9 * * *.What changed
CronScheduleHelper— newSplitTimeZonehelper strips an optionalCRON_TZ=prefix (case-insensitive), resolves it viaTimeZoneInfo.FindSystemTimeZoneById, and passes the zone to CronosGetNextOccurrence(from, tz), which is DST-aware out of the box. Unknown zones throwCronFormatExceptionwith a clear message instead of the confusing 'Minutes: Value must be a number' parse error.TryParsegains anout TimeZoneInfooverload;Describereports the actual zone instead of hardcoded 'UTC'.ReminderSchedule.CronExpression;ReminderManagerActor.ScheduleDefinitionAsyncre-parses it on every (re)schedule, so the zone survives restarts and reschedules for free.set_remindertool description andReminderScheduleParservalidation error now mention the prefix.Default behavior is unchanged: expressions without the prefix still evaluate in UTC.
Verified against pinned Cronos 0.13.0
Tests
New cases in
CronScheduleHelperTests: prefix parsing (valid/case-insensitive/missing zone/unknown zone), zone resolution, local-zone evaluation, DST spring-forward and fall-back transitions (Europe/Brussels 2026), unknown-zone rejection, no-prefix UTC regression, andDescribezone reporting. NewSetReminderToolTestscases: prefixed expression accepted and stored as-is, unknown zone rejected. Full Reminders suite: 144/144 passing.