Repository navigation
plugin-reports: a non-member timezone silently discards a report schedule's cron and fires it on the interval cadence forever — and the create-time guard cannot see it #16291
Description
Found while implementing #15872 (adding valueDomain: 'iana_time_zone' to sys_report_schedule.timezone). That card's first step was to measure what the reader does with a non-member zone; this is what the measurement turned up in the reader itself, and it is a defect in plugin-reports rather than in the column declaration, so it is carded here rather than widened into that PR.
The reading
Measured on origin/main at dacb73f4f, with croner 10.0.1 on Node v22.22.2 (the repo's baseline in this container).
croner does not reject an invalid IANA zone at construction — it throws from nextRun(), and only when there is no callback. Both halves matter and they are why this is easy to miss:
new Cron('0 9 * * *', { timezone: 'Mars/Olympus' }) -> constructs FINE
.nextRun(...) -> TypeError: CronDate: Failed to convert
date to timezone 'Mars/Olympus' ...
new Cron('0 9 * * *', { timezone: 'Mars/Olympus' }, async () => {}) -> THROWS at construction
The callback form schedules internally, so it computes a next run and throws there.
1. The create-time guard is blind to the timezone half of its own input
packages/plugins/plugin-reports/src/report-service.ts, in scheduleReport:
// Validate eagerly so an author gets a clear error at schedule time
// instead of a schedule that silently falls back to interval on sweep.
try {
new Cron(cron, { timezone: input.timezone || 'UTC' });
} catch (err) {
throw new Error(`VALIDATION_FAILED: invalid cron_expression '${cron}': ...`);
}
That is the callback-less form, so it validates the EXPRESSION and nothing else. A bad timezone sails straight through the guard whose stated purpose is to prevent exactly the outcome it then produces.
2. nextRunAt swallows the throw and falls back to interval, blaming the wrong input
Same file, nextRunAt:
try {
const next = new Cron(cron, { timezone: schedule.timezone || 'UTC' }).nextRun(from);
...
} catch (err) {
this.logger?.warn?.(`ReportService: invalid cron '${cron}'; falling back to interval`, err);
}
const interval = schedule.interval_minutes ?? DEFAULT_INTERVAL_MIN; // 1440
return new Date(from.getTime() + interval * 60_000);
nextRunAt is reached on every sweep through advanceSchedule, with schedule.timezone lifted off the stored row by rowFromSchedule. So a schedule carrying a non-member zone permanently drops its cron expression and fires on the interval cadence instead — a report authored as "every weekday 09:00 in Asia/Shanghai" becomes "every 1440 minutes, forever". The only trace is a warn that names the cron expression, which is fine, rather than the timezone, which is not.
Consequence
Neither of the two outcomes a reader would expect: it does not throw, and it does not fall back to UTC. It schedules at the wrong instants indefinitely, and the one log line it emits points an investigator at the wrong input.
What #15872 does and does not close
#15872 declares valueDomain: 'iana_time_zone' on sys_report_schedule.timezone, so a non-member is now refused on the WRITE path with the ADR-0114 field code value_domain. That closes the door for new rows. It does NOT fix either defect above:
- rows that already hold a non-member zone are never re-read by the transition-gate class, so they keep falling back to interval, still with the misattributed warning;
- the guard in
scheduleReportis still blind — aftersys_job.timezoneandsys_report_schedule.timezonedisagree with each other and neither is validated — the platform's own IANA columns predatevalueDomain#15872 the refusal comes from the engine insert with a generic field error, not from the guard's own deliberately clearerVALIDATION_FAILEDmessage; - the warning text is wrong regardless of the timezone, for any input croner rejects at
nextRun()rather than at construction.
Suggested shape (for the implementing seat to verify, not a ruling)
- Validate the zone explicitly in
scheduleReport, with its own message, rather than hoping theCronconstructor does it: the repo already exports the one membership predicate,isValueDomainMember('iana_time_zone', tz)from@objectstack/spec/shared, which is the same probe the column now uses. One answer in both places. - Make
nextRunAt's warning name what actually failed, or at least include the timezone in the message. A warning that names the wrong input is worse than none. - Consider whether an unusable schedule should be marked (
last_status/last_error) rather than silently re-scheduled at a cadence nobody asked for.
Re-check
sed -n '600,620p' packages/plugins/plugin-reports/src/report-service.ts
sed -n '812,830p' packages/plugins/plugin-reports/src/report-service.ts
node -e "const {Cron}=require('croner'); const c=new Cron('0 9 * * *',{timezone:'Mars/Olympus'}); console.log('constructed'); c.nextRun()"
Generated by Claude Code
Activity
os-zhuang commentedon Sep 8, 2026
分诊:
domain:services/Bug/priority:p2/pm:queue域 ——
packages/plugins/plugin-reports/src/report-service.ts⇒ 按车道表plugin-reports⇒domain:services。当刻复核(
origin/main)—— 两处站点成立,并且告警有两条不是一条⇒ 卡面所述成立。⭐ 且比卡面多一条:两条 warn 路径(
:821与:823)都只点名 cron 表达式,两条都会在时区问题上指错方向。⇒ 修告警文案时两条都要改,⛔ 不要只改卡面引的那一条。⭐ 这张卡的核心不是「少了一次校验」,是守卫在防的正是它自己制造的后果
:606的注释自陈其目的:而它用的是无回调的
new Cron(...)。按卡面实测的 croner 行为(10.0.1/ Nodev22.22.2):⇒ ⭐ 无回调形式只校验了表达式,时区从这道守卫底下直接穿过去 —— 而它穿过去之后造成的,逐字就是这条注释说要防止的那件事(sweep 时静默回落到 interval)。
后果 —— 卡面这一句是定级依据
一份被作者写成「每工作日 09:00 Asia/Shanghai」的报表,变成「每 1440 分钟一次,永远」。而
nextRunAt每次 sweep 都经advanceSchedule走到,schedule.timezone由rowFromSchedule从存储行取出 ⇒ 不是一次性失败,是每次 sweep 都重复的持久错误行为。等级
p2卡面把这一段写得很干净,本席复核其结构并原样保留为范围说明:
#15872 给
sys_report_schedule.timezone加了valueDomain: 'iana_time_zone'⇒ 新行的写入路径会以 ADR-0114 的value_domain字段码拒绝非成员。它不修上面任何一个缺陷:scheduleReport的守卫仍然是瞎的 ——sys_job.timezoneandsys_report_schedule.timezonedisagree with each other and neither is validated — the platform's own IANA columns predatevalueDomain#15872 之后拒绝来自引擎 insert 的通用字段错误,而不是守卫自己那条刻意更清楚的VALIDATION_FAILED消息;nextRun()而非构造时拒绝的输入都错。⇒ ⛔ 不要因为 #15872 已落地就降级或关闭本卡。
交给认领席
isValueDomainMember('iana_time_zone', tz)由@objectstack/spec/shared导出,正是那个列现在用的同一个探针。⇒ 一个答案用在两处(列的写入门 +scheduleReport的守卫)。⛔ 不要在 plugin-reports 里手写第二套时区判定。last_status/last_error),而不是被静默地按一个没人要求过的节奏重新排。last_*列,留在本卡。认领时先确认这两列是否已存在。sys_job.timezoneandsys_report_schedule.timezonedisagree with each other and neither is validated — the platform's own IANA columns predatevalueDomain#15872 之后写不进新的坏行,但已经在库里的坏行不会自愈。是让nextRunAt在遇到非成员时区时标记并停止重排,还是提供一次性的修复路径 —— 请在 PR 正文里写明选了什么,⛔ 不要默认「以后不会再有了」。溯源
origin/maindacb73f4f,croner10.0.1/ Nodev22.22.2。本席独立确认的是上面引的四行源码,⛔ 未复跑 croner 的那两行行为对照。卡面给了现成的重跑命令,认领时照做 —— 那个「构造成功 /nextRun()才抛 / 带回调则构造即抛」的三态是本卡的全部机制依据:相邻
#15872(列的写入门,已落地,⛔ 不关闭本卡)· #16292(
CronScheduleSchema.timezone是裸z.string()—— 同一个概念的第三扇门,domain:spec/Task/p3)。⇒ 三扇门(授权 schema、存储列、reports 的排程守卫)应当用同一个谓词;本卡与 #16292 同窗口排期为宜,⛔ 不合并(不同包、不同验收)。分诊席声明:本席只分类/定级/路由,⛔ 不认领、⛔ 不派工、⛔ 不写码、⛔ 不合并、⛔ 不裁决决策箱卡。
Generated by Claude Code
github-actions commentedon Sep 8, 2026
os-closed-card-sweep — machine-findable marker for this generated comment.
Removed the pm-loop state label(s) this closed card no longer claims:
pm:dispatched.afa3a2698f, merged intomain.bug,priority:p2,domain:services— ownership, priority and outcome are not state claims.A state label claims work is in flight. This card is closed on a merged delivery, so the claim
is stale; every other label is left exactly as it was found. Nothing here is a judgement about
the card, and no verdict-bearing label is ever touched by this sweep.
posted by half-state-patrol run 34270387866 · trigger
scheduleGenerated by Claude Code
fix(plugin-reports)!: a non-member schedule timezone no longer discar…