feat(reminders): support CRON_TZ prefix for timezone-aware cron schedules - #1789
Conversation
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. |
…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
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.
1d1d1a5 to
6f5bd61
Compare
Aaronontheweb
left a comment
There was a problem hiding this comment.
LGTM - going to do a follow-on PR for this
…nflicts Only conflict was the netclaw-operations skill version in SKILL.md. Resolved to 2.48.0 (merge-base 2.45.0; this branch 2.46.0; dev advanced to 2.47.0), a forward bump above dev so the CRON_TZ skill content keeps a clean version identity. CRON_TZ code and docs preserved; 76/76 CRON_TZ + SetReminderTool tests pass on the merged tree.
…#1886) * docs(reminders): plan scheduling-failure surfacing (Tier 1) Route reminder scheduling failures through the same alert, counter, and auto-disable path that execution failures use. Today the post-fire reschedule and the startup reconcile drop the failure, so a reminder can stay enabled and never fire without any alert. Artifacts: proposal, design, specs delta (netclaw-scheduling), tasks. The health-message-shape choice is parked in design.md as an open fork. * feat(reminders): surface scheduling failures loudly (Tier 1) Reminder scheduling failures were silent. The post-fire reschedule and the startup reconcile only logged a failed schedule, so a reminder could stay enabled and never fire with no alert and no failure count. Route both unattended reschedule sites through a new ReportScheduleFailureAsync, a sibling of the execution-failure path. It reuses the shared ConsecutiveFailures count, the FailurePauseThreshold, the notification sink, and the channel-notice helper. A scheduling failure now emits a ReminderScheduleFailed (Warning) alert and counts toward auto-disable. Crossing the threshold disables the reminder and emits the ReminderAutoDisabled (Critical) alert plus a channel notice. An unresolvable schedule never falls back to a different time; it fails loud. The shared count resets on a successful execution, not on a reschedule, so cron execution-failure accumulation is preserved. - add AlertType.ReminderScheduleFailed (appended; ordinal-stable) - wire post-fire reschedule and reconcile restore loop - update netclaw-operations skill (2.46.0 -> 2.47.0) - tests: 4 actor-level cases; full Reminders suite 136/136 Refs PRD-008 SCHED-007. OpenSpec change: surface-reminder-schedule-failures. * test(evals): add local-timezone CRON_TZ eval cases Two cases now that CRON_TZ (#1789) is in dev: - skill_cron_tz_timezone (Skill Discovery): the model surfaces the CRON_TZ prefix for a local-time schedule, recovered from the netclaw-operations skill. - grounding_cron_tz_schedule (Grounding): the model calls set_reminder AND carries CRON_TZ into the schedule, instead of silently converting to UTC. Pairs with this branch's scheduling-failure surfacing: an unresolvable CRON_TZ zone now fails loud and counts toward auto-disable rather than dying silently.
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.