refactor(core): unify cron expression authority - #1937
Merged
Conversation
Astro-Han
approved these changes
Aug 3, 2026
Astro-Han
left a comment
Contributor
There was a problem hiding this comment.
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/Numbercoercion, and DST fold/gap. - Migration completeness:
parsePlanReminderCronExpression/expandCronField/normalizeCronExpressionfully removed; no third cron authority remains; PR's own 17/17 tests pass (incl. DST subprocess test underTZ=America/New_York). - Behavior preserved bit-for-bit under the "don't change persisted behavior" constraint; named
PROFILE_POLICIEStable makes the previously implicit dialect fork explicit and testable.
Non-blocking P3 follow-ups (optional):
plan-reminders.ts:556— thecase 'unsatisfiable'branch is unreachable (plan profiles never setrejectImpossibleDates); add a comment or remove.matchesCronFieldis exported from the core public surface but hardcodes automation-v1 semantics; consider documenting the profile binding in its JSDoc for future core consumers.- 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.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
plan-reminders.tsandautomation-state.tseach 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-expressionauthority. Plan Reminder and Runtime Automation consume the same opaque compiled representation, while their scheduling responsibilities stay where they are: Plan still ownsstartAt, 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-v1keeps the numeric-only grammar, single-space field boundary, step ceiling, normalization errors, and bounded Plan search contract.automation-v1keeps ASCII-whitespace input, month/day aliases, Sunday0/7, Vixie DOM/DOW OR semantics, the eight-year sparse-date search, and persisted legacy coercions such as parseInt-prefixed tokens.*,15and legacy ranges whose effective set is empty, while allowing empty sets to return before the minute scan.computeNextCronFireandmatchesCronFieldremain available.Refs #1404
中文说明
plan-reminders.ts与automation-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 空白、月份/星期别名、Sunday0/7、Vixie DOM/DOW OR 语义、八年稀疏日期搜索,以及parseInt前缀等已持久化的历史 coercion。*,15以及 effective set 为空的历史 range 都保持原行为,同时空集合可以在分钟扫描前直接返回。computeNextCronFire与matchesCronField导出路径保持可用。Verification
npm --workspace @maka/core test— 715 passednpm --workspace @maka/runtime test— 2,695 passed, 3 skippednpm --workspace @maka/storage test— 1,023 passed, 3 skippednpm run typechecknpm run lint— 2,280 files checkednpm run format:check— 1,407 files checkedgit diff origin/main...HEAD --check@maka/core,@maka/core/cron-expression, and the compatibility exports from@maka/runtimefrom their built artifactsReview focus
PROFILE_POLICIESis the compatibility boundary. Please check that it records the two existing caller contracts without exposing parser internals to consumers.validationValuesand effectivevaluesintentionally differ for legacy inputs. Validation retains the old impossible-date authority; matching reproduces the values the retired matcher would actually select.