Skip to content

Commit 0491ac9

Browse files
committed
feat(platform-objects): validate sys_job.timezone and sys_report_schedule.timezone against the IANA domain
Both columns predate `valueDomain` and disagreed in three dimensions at once (length 100 vs 64, default none vs 'UTC', validation neither). This closes the validation dimension only: both now declare `valueDomain: 'iana_time_zone'`, the same declaration `sys_business_unit.timezone` / `sys_organization.timezone` carry, and the same shared `Intl.DateTimeFormat` membership probe. The reader measurement that decides what this is worth is recorded beside each declaration: `sys_report_schedule.timezone` is read back into croner by `ReportService.nextRunAt`, whose catch turned a non-member zone into a silent fall back to `interval_minutes` (the wrong instant, permanently), while `sys_job.timezone` is written and never read. Defaults and bounds are deliberately left unconverged and pinned as such. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01ARYe3yQTQCUFm5qPYNgKaJ
1 parent dacb73f commit 0491ac9

7 files changed

Lines changed: 217 additions & 7 deletions
Lines changed: 19 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,19 @@
1+
---
2+
'@objectstack/platform-objects': minor
3+
---
4+
5+
feat(platform-objects): `sys_job.timezone` and `sys_report_schedule.timezone` are validated against the IANA domain (#15872)
6+
7+
<!-- adr-0087: not-required (no-migration-prescription) A NON-BREAKING ADDITION of one field property to two existing columns. `valueDomain` is the `min`/`max`/`maxLength` transition-gate class: only a WRITTEN value is judged, a stored value outside a domain declared later is never re-read and survives unrelated edits, so `objectstack migrate meta` has nothing to rewrite. No metadata key, export, config field or stored shape is renamed, retired, re-typed or tombstoned; no column is added, dropped or re-bounded (`maxLength` is unchanged on both, deliberately), so boot schema-sync plans no DDL either. MIGRATION NOTE, stated rather than assumed: a deployment that already stored a non-IANA string in either column keeps it and reads it back unchanged; what changes is that the next WRITE of such a value is refused with the ADR-0114 field error code `value_domain`. For `sys_report_schedule` that refusal is the point — see the behaviour note below. -->
8+
9+
The platform's two oldest IANA time-zone columns predate `valueDomain` and disagreed with each other in three dimensions at once — length (100 vs 64), default (none vs `'UTC'`) and validation (neither). This closes the third: both now declare `valueDomain: 'iana_time_zone'`, the same declaration and the same `Intl.DateTimeFormat` membership probe that `sys_business_unit.timezone` and `sys_organization.timezone` carry (#14238). Four columns, one spelling of "is this a real zone".
10+
11+
**What it was worth, measured before the fix rather than assumed.** The two columns are not equally exposed, and only one of them was dangerous.
12+
13+
- `sys_report_schedule.timezone` is read back and handed to a scheduler. `ReportService.nextRunAt` calls `new Cron(cron, { timezone }).nextRun(from)`, and croner does not reject a non-member zone at construction when there is no callback — it throws from `nextRun()`. That throw was caught and turned into a fall back to `interval_minutes`. So a typo'd zone silently discarded the cron expression: an admin's "every weekday 09:00 Asia/Shanghai" became "every 1440 minutes, forever", logged only as `invalid cron '<expr>'` — a warning naming the wrong input, because the expression was fine. Not a throw and not a fall back to UTC: the wrong instant, permanently. Refusing the write is what closes it. (`scheduleReport`'s eager create-time guard did not catch it either: it constructs a callback-less `Cron` and is blind to exactly this half of its own input. That is a separate defect in `plugin-reports`, carded, not fixed here.)
14+
- `sys_job.timezone` is written and never read. `DbJobAdapter` mirrors the in-memory schedule onto the row; its three `sys_job` read sites take `id` / `run_count` / `failure_count` only. The zone the scheduler honours never travels through this column, and `DbJobAdapter.schedule` awaits the cron adapter before it upserts the row, so a non-member cannot even reach the column that way — croner constructed WITH a callback throws, and `AppPlugin` reports it as `Background job FAILED TO SCHEDULE — it will never run`. The door this declaration closes there is the other one: a direct write from Studio, REST or a script, which had no validation at all.
15+
16+
**What is deliberately NOT converged**, and is pinned so that staying unconverged is a decision rather than a drift someone repairs by reflex:
17+
18+
- **the defaults still differ.** A default here is a consumer semantic, not a shape question. `sys_report_schedule` documents and implements a UTC default; `sys_job` has no reader at all, and minting one would change what an unset row means.
19+
- **the bounds still differ (100 vs 64).** `maxLength` is not only a write bound — it reaches DDL, and narrowing a physical `varchar(100)` is `driver-sql`'s `narrow_varchar` op at severity `error`, category destructive ("narrowing may truncate"). What the column physically holds in a deployment is not readable from the repo, so the convergence is a separate decision and #15872 stays open on it. Note what the domain declaration already costs the wider bound: no member is longer than 32 characters on the current Node baseline, so 100 now admits nothing 64 would not.
Lines changed: 121 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,121 @@
1+
// Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license.
2+
3+
/**
4+
* #15872 — the platform's two OLDER IANA columns, `sys_job.timezone` and
5+
* `sys_report_schedule.timezone`, predate `valueDomain` and disagreed with each
6+
* other in three dimensions at once (length 100 vs 64, default none vs `'UTC'`,
7+
* validation none vs none). This file pins what that card actually changed and,
8+
* just as deliberately, what it did NOT.
9+
*
10+
* CLOSED here — validation. Both columns now declare
11+
* `valueDomain: 'iana_time_zone'`, the same declaration the ruled pair
12+
* `sys_business_unit.timezone` / `sys_organization.timezone` carries (#14238,
13+
* pinned in `identity/org-hierarchy-timezone.test.ts`). Four columns, one
14+
* membership predicate.
15+
*
16+
* LEFT ALONE, and pinned so that staying alone is a decision rather than a
17+
* drift someone repairs by reflex:
18+
*
19+
* - the DEFAULTS still differ, because a default here is a CONSUMER semantic,
20+
* not a shape question. `sys_report_schedule` documents "default UTC" and its
21+
* reader falls back to `'UTC'`; `sys_job` says nothing, and giving it one
22+
* would change what an unset row means. The ruled pair, for its own reasons,
23+
* has none on either column — so "all four agree" is NOT the invariant, and
24+
* a test asserting it would be asserting a bug.
25+
* - the BOUNDS still differ (100 vs 64). `maxLength` is not only a write bound:
26+
* it reaches DDL, and narrowing a physical `varchar(100)` is `driver-sql`'s
27+
* `narrow_varchar` op at severity `error`, category destructive. What the
28+
* column physically holds in a deployment is not readable from the repo, so
29+
* the convergence is a separate decision and #15872 stays open on it.
30+
*
31+
* The reader measurement that decided the card's severity is recorded beside
32+
* each declaration, not here: the `sys_job` column is written and never read,
33+
* while the `sys_report_schedule` column is read back into croner by
34+
* `ReportService.nextRunAt`, whose catch turned a non-member zone into a silent
35+
* fall back to `interval_minutes` — the wrong instant, forever.
36+
*/
37+
38+
import { describe, it, expect } from 'vitest';
39+
import { isValueDomainMember } from '@objectstack/spec/shared';
40+
import { SysJob } from './sys-job.object';
41+
import { SysReportSchedule } from './sys-report-schedule.object';
42+
43+
type ColumnShape = {
44+
type?: unknown;
45+
required?: unknown;
46+
maxLength?: unknown;
47+
valueDomain?: unknown;
48+
defaultValue?: unknown;
49+
};
50+
51+
const jobColumn = () => (SysJob.fields as Record<string, ColumnShape>).timezone;
52+
const scheduleColumn = () => (SysReportSchedule.fields as Record<string, ColumnShape>).timezone;
53+
54+
describe('#15872 — the platform\'s two older IANA time-zone columns', () => {
55+
it('reads the real declarations, not an empty probe', () => {
56+
// Vacuity control: a renamed column or a changed export would otherwise let
57+
// every assertion below pass over `undefined`.
58+
expect(SysJob.name).toBe('sys_job');
59+
expect(SysReportSchedule.name).toBe('sys_report_schedule');
60+
expect(jobColumn()).toBeTypeOf('object');
61+
expect(scheduleColumn()).toBeTypeOf('object');
62+
});
63+
64+
it.each([
65+
['sys_job', jobColumn],
66+
['sys_report_schedule', scheduleColumn],
67+
])('%s.timezone is an optional text column validated against the IANA domain', (_object, column) => {
68+
const c = column();
69+
// `VALUE_DOMAIN_FIELD_TYPES` is `{text}`, so the declaration below is also
70+
// the reason the type must stay `text`.
71+
expect(c.type).toBe('text');
72+
expect(c.required).toBe(false);
73+
expect(c.valueDomain).toBe('iana_time_zone');
74+
});
75+
76+
it('the DEFAULTS deliberately still differ — a default here is a consumer semantic', () => {
77+
// ⛔ Not a tidy-up target. `sys_report_schedule`'s reader documents and
78+
// implements a UTC default; `sys_job` has no reader at all, and minting one
79+
// would give "unset" a new meaning on rows that predate it.
80+
expect(scheduleColumn().defaultValue).toBe('UTC');
81+
expect('defaultValue' in jobColumn()).toBe(false);
82+
});
83+
84+
it('the BOUNDS deliberately still differ — converging them is a DDL question, not a shape one', () => {
85+
// If someone converges these, they owe the reading #15872 could not take:
86+
// what the physical column holds. Red here is the prompt to go and take it.
87+
expect(jobColumn().maxLength).toBe(100);
88+
expect(scheduleColumn().maxLength).toBe(64);
89+
});
90+
91+
it('the declared domain refuses every non-member this card was filed over', () => {
92+
// Asked of the predicate the write path calls (`isValueDomainMember`) under
93+
// the domain the columns actually declare — never a re-implementation.
94+
const domain = jobColumn().valueDomain as 'iana_time_zone';
95+
expect(domain).toBe(scheduleColumn().valueDomain);
96+
// The card's own three examples. `Mars/Olympus` is shape-valid and
97+
// nonexistent, `UTC+8` and `China Standard Time` are the two spellings a
98+
// human reaches for that the tzdb does not carry.
99+
expect(isValueDomainMember(domain, 'Mars/Olympus')).toBe(false);
100+
expect(isValueDomainMember(domain, 'UTC+8')).toBe(false);
101+
expect(isValueDomainMember(domain, 'China Standard Time')).toBe(false);
102+
// …and still admits what both columns must keep taking, `UTC` included —
103+
// which `Intl.supportedValuesOf('timeZone')` omits, so a column judged
104+
// against the enumeration would refuse `sys_report_schedule`'s own default.
105+
expect(isValueDomainMember(domain, 'UTC')).toBe(true);
106+
expect(isValueDomainMember(domain, 'Asia/Shanghai')).toBe(true);
107+
expect(isValueDomainMember(domain, scheduleColumn().defaultValue as string)).toBe(true);
108+
});
109+
110+
it('both bounds admit every zone the runtime enumerates, so neither refuses a legal value', () => {
111+
// The smaller bound is the one that could bite; assert against both so a
112+
// future ICU that enumerates a longer name reds here rather than silently
113+
// refusing a legal zone at the write seam.
114+
// `Intl.supportedValuesOf` is ES2022; the package's `lib` predates it, so
115+
// the call is typed here rather than the whole program's lib widened.
116+
const intl = Intl as unknown as { supportedValuesOf(key: 'timeZone'): string[] };
117+
const longest = Math.max(...intl.supportedValuesOf('timeZone').map((z) => z.length));
118+
expect(longest).toBeLessThanOrEqual(scheduleColumn().maxLength as number);
119+
expect(longest).toBeLessThanOrEqual(jobColumn().maxLength as number);
120+
});
121+
});

packages/platform-objects/src/audit/sys-job.object.ts

Lines changed: 34 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -61,10 +61,44 @@ export const SysJob = ObjectSchema.create({
6161
group: 'Schedule',
6262
}),
6363

64+
// [#15872] Validated on write by `valueDomain: 'iana_time_zone'` — the same
65+
// declaration `sys_business_unit.timezone` / `sys_organization.timezone`
66+
// carry (#14238), and the same shared `Intl.DateTimeFormat` probe, never the
67+
// `Intl.supportedValuesOf('timeZone')` enumeration (which omits `UTC`).
68+
// Written values only: the `min`/`max`/`maxLength` transition-gate class, so
69+
// a stored non-member is never re-read and no migration is owed.
70+
//
71+
// WHAT READS THIS COLUMN, measured on #15872 before the declaration was
72+
// added, because it decides what the declaration is worth: NOTHING does.
73+
// `DbJobAdapter` writes it (`upsertJobRow`, `schedule.timezone ?? null`) and
74+
// its three `sys_job` read sites take `id` / `run_count` / `failure_count`
75+
// only — the tree's one `row.timezone` read belongs to `sys_report_schedule`.
76+
// The value the scheduler actually honours travels in memory
77+
// (`toBoundaryJobSchedule` -> `CronJobAdapter.schedule` -> croner), and
78+
// `DbJobAdapter.schedule` awaits that call BEFORE `upsertJobRow`, so a
79+
// non-member cannot reach this column through the scheduler at all: croner
80+
// constructed WITH a callback throws on a non-member zone, `AppPlugin`
81+
// catches it per job as `Background job FAILED TO SCHEDULE — it will never
82+
// run` (error + `jobScheduleFailuresTotal`), and the row is never written.
83+
// The door this declaration actually closes is the OTHER one: a direct write
84+
// to the object (Studio, REST, a script), which had no validation whatever.
85+
//
86+
// ⚠️ `maxLength` deliberately still says 100 while `sys_report_schedule`
87+
// says 64. Converging it is the card's third dimension and is NOT landed
88+
// here: `maxLength` is not only a write bound, it reaches DDL — narrowing a
89+
// physical `varchar(100)` produces `driver-sql`'s `narrow_varchar` op at
90+
// severity `error`, category destructive ("narrowing may truncate",
91+
// `os migrate apply --allow-destructive`). What this column physically holds
92+
// in a deployment cannot be read from the repo, and 「IANA names are short」
93+
// is an argument about the domain, not a reading of the data. Left to a
94+
// separate decision (#15872 stays open on that row). Note what the line
95+
// above already costs it: no `valueDomain` member is longer than 32
96+
// characters on this Node baseline, so 100 now admits nothing 64 would not.
6497
timezone: Field.text({
6598
label: 'Timezone',
6699
required: false,
67100
maxLength: 100,
101+
valueDomain: 'iana_time_zone',
68102
group: 'Schedule',
69103
}),
70104

packages/platform-objects/src/audit/sys-report-schedule.object.ts

Lines changed: 31 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -78,11 +78,42 @@ export const SysReportSchedule = ObjectSchema.create({
7878
group: 'Schedule',
7979
}),
8080

81+
// [#15872] Validated on write by `valueDomain: 'iana_time_zone'` — the same
82+
// declaration `sys_business_unit.timezone` / `sys_organization.timezone`
83+
// carry (#14238), and the same shared `Intl.DateTimeFormat` probe, never the
84+
// `Intl.supportedValuesOf('timeZone')` enumeration (which omits `UTC`, this
85+
// column's own default). Written values only (the `min`/`max`/`maxLength`
86+
// transition-gate class), so a stored non-member survives and no migration
87+
// is owed.
88+
//
89+
// WHY THIS COLUMN IS THE SHARP ONE, measured on #15872: unlike
90+
// `sys_job.timezone`, this value IS read back and handed to a scheduler.
91+
// `ReportService.rowFromSchedule` lifts it off the row and `nextRunAt` calls
92+
// `new Cron(cron, { timezone }).nextRun(from)`. croner (10.0.1) does not
93+
// reject a non-member zone when it is constructed WITHOUT a callback — it
94+
// throws from `nextRun()` — and `nextRunAt` CATCHES that throw and falls
95+
// back to `from + interval_minutes`. So before this line, a typo'd zone on a
96+
// cron schedule silently discarded the cron: an admin's "every weekday 09:00
97+
// Asia/Shanghai" became "every 1440 minutes, forever", logged only as
98+
// `invalid cron '<expr>'` — a warning that names the wrong input, since the
99+
// expression was fine. Neither a throw nor a fall back to UTC: the wrong
100+
// instant, permanently, which is the outcome this card was told to escalate
101+
// on. `scheduleReport`'s eager create-time guard does not catch it either;
102+
// it constructs a callback-less `Cron` and so is blind to exactly this half
103+
// of its own input. Refusing the write is what closes it.
104+
//
105+
// `maxLength: 64` and `defaultValue: 'UTC'` are BOTH unchanged. The bound is
106+
// already the value #14238 justified (twice the domain's real ceiling: the
107+
// enumeration's longest name is 30 characters on this Node baseline, the
108+
// longest tzdb link 32). The default is a consumer semantic — this reader
109+
// documents "default UTC" and falls back to `'UTC'` in four places — and is
110+
// deliberately NOT converged with `sys_job`, which has none.
81111
timezone: Field.text({
82112
label: 'Timezone',
83113
required: false,
84114
maxLength: 64,
85115
defaultValue: 'UTC',
116+
valueDomain: 'iana_time_zone',
86117
group: 'Schedule',
87118
}),
88119

packages/platform-objects/src/identity/org-hierarchy-timezone.test.ts

Lines changed: 5 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -20,8 +20,11 @@
2020
* no default on either. The card's thesis is that every author invents this
2121
* column differently, and the platform's own two precedents
2222
* (`sys_job.timezone`: 100, no default; `sys_report_schedule.timezone`: 64,
23-
* default `UTC`; neither validated) already disagree in three dimensions.
24-
* The ruled pair must not become a third and a fourth spelling;
23+
* default `UTC`) disagreed in three dimensions. #15872 closed the third by
24+
* giving both the same `valueDomain` this pair carries — pinned next to
25+
* them in `audit/platform-iana-timezone-columns.test.ts` — and left length
26+
* and default alone. The ruled pair must not become a third and a fourth
27+
* spelling;
2528
* 3. the declared domain admits `UTC`, the fallback the contract names for a
2629
* wholly unset chain, and the declared bound admits every zone the runtime
2730
* enumerates. Why the first is not automatic — `Intl.supportedValuesOf`

packages/platform-objects/src/identity/sys-business-unit.object.ts

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -167,8 +167,9 @@ export const SysBusinessUnit = ObjectSchema.create({
167167
// pairs a bound with the `UTC` default; the enumeration's longest name on
168168
// the repo's Node baseline is 30 characters and the tzdb caps each path
169169
// component at 14, so 64 is twice the domain's real ceiling and the smaller
170-
// of the two precedents (`sys_job.timezone` says 100, neither declares a
171-
// domain — the residue card). No `defaultValue`, deliberately: an explicit
170+
// of the two precedents (`sys_job.timezone` still says 100 — #15872 gave
171+
// both precedents this same `valueDomain` and deliberately left that one
172+
// bound unconverged). No `defaultValue`, deliberately: an explicit
172173
// default here would mean "stop inheriting", which is the opposite of what
173174
// an unset unit means.
174175
timezone: Field.text({

packages/platform-objects/src/identity/sys-organization.object.ts

Lines changed: 4 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -252,9 +252,10 @@ export const SysOrganization = ObjectSchema.create({
252252
// `sys_business_unit.timezone` by design (`text`, optional, `maxLength: 64`,
253253
// `valueDomain: 'iana_time_zone'`, no default): the card's thesis is that
254254
// every author invents this column differently, and the platform's own two
255-
// precedents (`sys_job`, `sys_report_schedule`) already disagree on length,
256-
// default and validation — the ruled pair is one spelling, pinned in
257-
// `org-hierarchy-timezone.test.ts`.
255+
// precedents (`sys_job`, `sys_report_schedule`) disagreed on length, default
256+
// AND validation — #15872 closed the validation dimension by giving both
257+
// this same `valueDomain`, and left the other two as each reader expects.
258+
// The ruled pair is one spelling, pinned in `org-hierarchy-timezone.test.ts`.
258259
timezone: Field.text({
259260
label: 'Timezone',
260261
required: false,

0 commit comments

Comments
 (0)