Skip to content

fix(proactive_engine): keep cron without tz on local DST rules - #1196

Open
pcbeingused333 wants to merge 2 commits into
EverMind-AI:mainfrom
pcbeingused333:fix/cron-local-tz-dst
Open

pcbeingused333 wants to merge 2 commits into
EverMind-AI:mainfrom
pcbeingused333:fix/cron-local-tz-dst

Conversation

@pcbeingused333

Copy link
Copy Markdown
Contributor

Summary

_compute_next_run used datetime.now().astimezone().tzinfo when a cron schedule had no tz (the default, since cron.default_timezone is unset). That is a fixed UTC offset taken from "now", so croniter ignored DST for the next occurrence. With TZ=America/New_York, 0 9 * * * computed from 2027-03-13T15:00Z returned 10:00 EDT instead of 09:00, so a daily reminder fired an hour late after the change.

With no tz, the schedule now uses a naive local datetime.fromtimestamp(...), so the system zone's DST rules apply. If the result falls in the repeated hour of a fall-back and is not after now, it steps to the next slot, so the job fires once and is never scheduled in the past. Schedules with an explicit tz are unchanged.

Type

  • Fix
  • Feature
  • Docs
  • CI / tooling
  • Refactor
  • Other

Verification

  • uv run pytest tests/test_cron_service_validation.py -q: 17 passed. test_cron_without_tz_follows_local_dst fails without the change.

  • ruff check and ruff format --check (v0.15.12, as in pre-commit) on the touched files: clean.

  • Relevant tests pass locally

  • Relevant lint / type checks pass locally

  • User-facing docs or screenshots are updated when needed

Risk

  • Security impact considered
  • Backward compatibility considered
  • Rollback path is clear for risky changes

Related Issues

N/A

Co-authored-by: Claude (claude-sonnet-5-5) noreply@anthropic.com

pcbeingused333 and others added 2 commits November 6, 2026 12:10
A cron schedule with no explicit timezone used the fixed UTC offset of
"now", so croniter computed the next run with a stale offset after a DST
change (09:00 became 10:00). Use a naive local datetime so the system
zone's DST rules apply.

Co-authored-by: Claude (claude-sonnet-5-5) <noreply@anthropic.com>
A naive wall time inside the repeated hour of a DST fall-back maps to
its first occurrence, which can already be past. Step to the next slot
so the job fires once instead of being scheduled before now.

Co-authored-by: Claude (claude-sonnet-5-5) <noreply@anthropic.com>
@LivXue

LivXue commented Nov 6, 2026

Copy link
Copy Markdown
Member

Windows compatibility review — PR #1196

Reviewed head cc9a8a6547b3252eb502bd18581c7eb52e1cc2e2 against base 8448cc6280923a7b20525af259a3b6d465844c42 (2 files, +50/−3: raven/proactive_engine/schedulers/cron/service.py and tests/test_cron_service_validation.py). The full diff and the surrounding _compute_next_run code were read.

Finding

1. Medium (Windows test failure, reproduced): both new tests call the POSIX-only time.tzset() unconditionally, so the Windows test suite goes red. time.tzset does not exist on Windows; the calls at tests/test_cron_service_validation.py:134 and :151 raise AttributeError: module 'time' has no attribute 'tzset' before either test reaches the code under test. Run on this native Windows host at this head: uv run --no-sync pytest tests/test_cron_service_validation.py -q → 2 failed, 15 passed; both failures are that AttributeError. The same file at base 8448cc628 has neither test and passes on Windows, so the regression is introduced here. This repository already guards the identical need — a process-wide zone move for a test — with @pytest.mark.skipif(not hasattr(time, "tzset"), reason="moving the process zone needs time.tzset") in tests/test_acp_journal.py:263 and tests/test_subagent_acp.py:5023. Remedy: add the same skipif marker to both new tests (or a module-level pytestmark), so Windows skips them instead of failing.

The production change itself looks correct on Windows: with no tz, the naive local datetime path now applies the OS zone's rules to every occurrence. Probe at this head on this machine: _compute_next_run(CronSchedule(kind='cron', expr='0 9 * * *'), now_ms) returned 2026-11-07T09:00:00 local, in the future, with no exception. datetime.fromtimestamp() / naive datetime.timestamp() go through the Windows CRT's local-time database, which carries the configured zone's DST rules.

Verification (native Windows 11, isolated detached worktree D:/tmp/rv-1196)

  • uv sync --frozen --offline --all-extras --no-extra segmenter --no-extra browser --dev, then uv run --no-sync pytest tests/test_cron_service_validation.py -q → 2 failed, 15 passed (both failures are the time.tzset AttributeError above).
  • Base comparison: git show 8448cc628:tests/test_cron_service_validation.py contains neither new test and no tzset call; that file passes on Windows.
  • Production probe above: no-tz cron computes a future local 09:00 without error.

Gap: this host's system zone is China Standard Time, which has no DST, so the actual across-DST behavior could not be exercised natively here; the DST assertion is only demonstrated by the PR's Linux run. The test failure, however, is fully reproduced on Windows.

This branch has not been deployed

No deployments
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants