Skip to content

refactor(core): unify cron expression authority - #1937

Merged
Astro-Han merged 5 commits into
apache:mainfrom
me2seeks:refactor/1404-cron-unification
Aug 3, 2026
Merged

refactor(core): unify cron expression authority#1937
Astro-Han merged 5 commits into
apache:mainfrom
me2seeks:refactor/1404-cron-unification

Conversation

@me2seeks

@me2seeks me2seeks commented Aug 3, 2026

Copy link
Copy Markdown
Contributor

Summary

plan-reminders.ts and automation-state.ts each owned a five-field cron implementation, and Automation additionally parsed the same field twice with different validation and matching rules.

This PR moves cron parsing, matching, and bounded next-occurrence search into one compiled @maka/core/cron-expression authority. Plan Reminder and Runtime Automation consume the same opaque compiled representation, while their scheduling responsibilities stay where they are: Plan still owns startAt, its 366-day horizon, and user-facing validation errors; Runtime still owns Automation scheduling and jitter, and the existing persistence and lifecycle boundaries are unchanged.

The two callers did not previously accept exactly the same dialect, so the shared module uses named compatibility profiles instead of silently normalizing their behavior:

  • plan-reminder-v1 keeps the numeric-only grammar, single-space field boundary, step ceiling, normalization errors, and bounded Plan search contract.
  • automation-v1 keeps ASCII-whitespace input, month/day aliases, Sunday 0/7, Vixie DOM/DOW OR semantics, the eight-year sparse-date search, and persisted legacy coercions such as parseInt-prefixed tokens.
  • Automation's retired validator/matcher disagreement is handled by one parser that produces a private validation projection and one effective match set. The validation projection is used only for the existing impossible-date gate; occurrence search consumes only the compiled effective set. This preserves existing schedules such as *,15 and legacy ranges whose effective set is empty, while allowing empty sets to return before the minute scan.
  • The existing Runtime exports computeNextCronFire and matchesCronField remain available.

Refs #1404

中文说明

plan-reminders.tsautomation-state.ts 原本各自维护一套五字段 Cron 实现;Automation 内部还会用不同规则对同一个字段分别做校验和匹配。

本 PR 将 Cron 解析、匹配以及有界的下一次触发时间搜索统一到 @maka/core/cron-expression。Plan Reminder 与 Runtime Automation 现在消费同一个不透明的编译结果,但原有职责边界不变:Plan 继续负责 startAt、366 天搜索窗口和面向用户的校验错误;Runtime 继续负责 Automation 调度和 jitter,现有持久化及生命周期边界不变。

由于两个调用方原本接受的 Cron 方言并不完全相同,共享模块使用具名兼容 profile,而不是静默改变既有行为:

  • plan-reminder-v1 保留仅数字语法、单空格字段边界、step 上限、规范化错误和 Plan 的有界搜索行为。
  • automation-v1 保留 ASCII 空白、月份/星期别名、Sunday 0/7、Vixie DOM/DOW OR 语义、八年稀疏日期搜索,以及 parseInt 前缀等已持久化的历史 coercion。
  • 旧 Automation validator/matcher 的差异由同一个 parser 处理:一次解析产生私有 validation projection 和唯一的 effective match set。validation projection 只用于保留既有 impossible-date 判断;下一次触发时间搜索只消费编译后的 effective set。因此,*,15 以及 effective set 为空的历史 range 都保持原行为,同时空集合可以在分钟扫描前直接返回。
  • Runtime 现有的 computeNextCronFirematchesCronField 导出路径保持可用。

Verification

  • npm --workspace @maka/core test — 715 passed
  • npm --workspace @maka/runtime test — 2,695 passed, 3 skipped
  • npm --workspace @maka/storage test — 1,023 passed, 3 skipped
  • npm run typecheck
  • npm run lint — 2,280 files checked
  • npm run format:check — 1,407 files checked
  • git diff origin/main...HEAD --check
  • loaded @maka/core, @maka/core/cron-expression, and the compatibility exports from @maka/runtime from their built artifacts
  • differential audit against the retired Automation field matcher — 216,942 comparisons with no mismatch

Review focus

  • PROFILE_POLICIES is the compatibility boundary. Please check that it records the two existing caller contracts without exposing parser internals to consumers.
  • Automation's validationValues and effective values intentionally differ for legacy inputs. Validation retains the old impossible-date authority; matching reproduces the values the retired matcher would actually select.
  • The empty-effective-set fast path follows the same Vixie DOM/DOW truth table as normal matching, so an empty DOM can still fire through a restricted DOW and vice versa.
  • Next-occurrence search advances in epoch minutes to remain strictly after the input across local DST folds. Runtime scheduling, jitter, persistence, and lifecycle ownership are outside this change.

@Astro-Han Astro-Han left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Review: MERGE-READY ✅

DeepSeek review of the full diff (8 files, +941/−501) with independent verification:

  • Differential fuzz: 150,000+ cases across automation next-fire / plan next-run / matchesCronField (reject/nomatch/fire three-state) — 0 mismatches against the old implementation, including sparse leap days, Vixie OR, impossible-date fast-reject, parseInt/Number coercion, and DST fold/gap.
  • Migration completeness: parsePlanReminderCronExpression / expandCronField / normalizeCronExpression fully removed; no third cron authority remains; PR's own 17/17 tests pass (incl. DST subprocess test under TZ=America/New_York).
  • Behavior preserved bit-for-bit under the "don't change persisted behavior" constraint; named PROFILE_POLICIES table makes the previously implicit dialect fork explicit and testable.

Non-blocking P3 follow-ups (optional):

  1. plan-reminders.ts:556 — the case 'unsatisfiable' branch is unreachable (plan profiles never set rejectImpossibleDates); add a comment or remove.
  2. matchesCronField is exported from the core public surface but hardcodes automation-v1 semantics; consider documenting the profile binding in its JSDoc for future core consumers.
  3. Consider committing the frozen old-matcher snapshot + differential regression test so the equivalence surface stays locked (the 216,942-case audit was run once, not checked in).

None block merging.

@Astro-Han
Astro-Han merged commit 91797f3 into apache:main Aug 3, 2026
5 checks passed
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