Skip to content
Draft
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
19 changes: 19 additions & 0 deletions .changeset/platform-iana-timezone-columns.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,19 @@
---
'@objectstack/platform-objects': minor
---

feat(platform-objects): `sys_job.timezone` and `sys_report_schedule.timezone` are validated against the IANA domain (#15872)

<!-- 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. -->

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".

**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.

- `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.)
- `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.

**What is deliberately NOT converged**, and is pinned so that staying unconverged is a decision rather than a drift someone repairs by reflex:

- **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.
- **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.
Original file line number Diff line number Diff line change
@@ -0,0 +1,121 @@
// Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license.

/**
* #15872 — the platform's two OLDER IANA columns, `sys_job.timezone` and
* `sys_report_schedule.timezone`, predate `valueDomain` and disagreed with each
* other in three dimensions at once (length 100 vs 64, default none vs `'UTC'`,
* validation none vs none). This file pins what that card actually changed and,
* just as deliberately, what it did NOT.
*
* CLOSED here — validation. Both columns now declare
* `valueDomain: 'iana_time_zone'`, the same declaration the ruled pair
* `sys_business_unit.timezone` / `sys_organization.timezone` carries (#14238,
* pinned in `identity/org-hierarchy-timezone.test.ts`). Four columns, one
* membership predicate.
*
* LEFT ALONE, and pinned so that staying alone is a decision rather than a
* drift someone repairs by reflex:
*
* - the DEFAULTS still differ, because a default here is a CONSUMER semantic,
* not a shape question. `sys_report_schedule` documents "default UTC" and its
* reader falls back to `'UTC'`; `sys_job` says nothing, and giving it one
* would change what an unset row means. The ruled pair, for its own reasons,
* has none on either column — so "all four agree" is NOT the invariant, and
* a test asserting it would be asserting a bug.
* - 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. 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.
*
* The reader measurement that decided the card's severity is recorded beside
* each declaration, not here: the `sys_job` column is written and never read,
* while the `sys_report_schedule` column 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, forever.
*/

import { describe, it, expect } from 'vitest';
import { isValueDomainMember } from '@objectstack/spec/shared';
import { SysJob } from './sys-job.object';
import { SysReportSchedule } from './sys-report-schedule.object';

type ColumnShape = {
type?: unknown;
required?: unknown;
maxLength?: unknown;
valueDomain?: unknown;
defaultValue?: unknown;
};

const jobColumn = () => (SysJob.fields as Record<string, ColumnShape>).timezone;
const scheduleColumn = () => (SysReportSchedule.fields as Record<string, ColumnShape>).timezone;

describe('#15872 — the platform\'s two older IANA time-zone columns', () => {
it('reads the real declarations, not an empty probe', () => {
// Vacuity control: a renamed column or a changed export would otherwise let
// every assertion below pass over `undefined`.
expect(SysJob.name).toBe('sys_job');
expect(SysReportSchedule.name).toBe('sys_report_schedule');
expect(jobColumn()).toBeTypeOf('object');
expect(scheduleColumn()).toBeTypeOf('object');
});

it.each([
['sys_job', jobColumn],
['sys_report_schedule', scheduleColumn],
])('%s.timezone is an optional text column validated against the IANA domain', (_object, column) => {
const c = column();
// `VALUE_DOMAIN_FIELD_TYPES` is `{text}`, so the declaration below is also
// the reason the type must stay `text`.
expect(c.type).toBe('text');
expect(c.required).toBe(false);
expect(c.valueDomain).toBe('iana_time_zone');
});

it('the DEFAULTS deliberately still differ — a default here is a consumer semantic', () => {
// ⛔ Not a tidy-up target. `sys_report_schedule`'s reader documents and
// implements a UTC default; `sys_job` has no reader at all, and minting one
// would give "unset" a new meaning on rows that predate it.
expect(scheduleColumn().defaultValue).toBe('UTC');
expect('defaultValue' in jobColumn()).toBe(false);
});

it('the BOUNDS deliberately still differ — converging them is a DDL question, not a shape one', () => {
// If someone converges these, they owe the reading #15872 could not take:
// what the physical column holds. Red here is the prompt to go and take it.
expect(jobColumn().maxLength).toBe(100);
expect(scheduleColumn().maxLength).toBe(64);
});

it('the declared domain refuses every non-member this card was filed over', () => {
// Asked of the predicate the write path calls (`isValueDomainMember`) under
// the domain the columns actually declare — never a re-implementation.
const domain = jobColumn().valueDomain as 'iana_time_zone';
expect(domain).toBe(scheduleColumn().valueDomain);
// The card's own three examples. `Mars/Olympus` is shape-valid and
// nonexistent, `UTC+8` and `China Standard Time` are the two spellings a
// human reaches for that the tzdb does not carry.
expect(isValueDomainMember(domain, 'Mars/Olympus')).toBe(false);
expect(isValueDomainMember(domain, 'UTC+8')).toBe(false);
expect(isValueDomainMember(domain, 'China Standard Time')).toBe(false);
// …and still admits what both columns must keep taking, `UTC` included —
// which `Intl.supportedValuesOf('timeZone')` omits, so a column judged
// against the enumeration would refuse `sys_report_schedule`'s own default.
expect(isValueDomainMember(domain, 'UTC')).toBe(true);
expect(isValueDomainMember(domain, 'Asia/Shanghai')).toBe(true);
expect(isValueDomainMember(domain, scheduleColumn().defaultValue as string)).toBe(true);
});

it('both bounds admit every zone the runtime enumerates, so neither refuses a legal value', () => {
// The smaller bound is the one that could bite; assert against both so a
// future ICU that enumerates a longer name reds here rather than silently
// refusing a legal zone at the write seam.
// `Intl.supportedValuesOf` is ES2022; the package's `lib` predates it, so
// the call is typed here rather than the whole program's lib widened.
const intl = Intl as unknown as { supportedValuesOf(key: 'timeZone'): string[] };
const longest = Math.max(...intl.supportedValuesOf('timeZone').map((z) => z.length));
expect(longest).toBeLessThanOrEqual(scheduleColumn().maxLength as number);
expect(longest).toBeLessThanOrEqual(jobColumn().maxLength as number);
});
});
34 changes: 34 additions & 0 deletions packages/platform-objects/src/audit/sys-job.object.ts
Original file line number Diff line number Diff line change
Expand Up @@ -61,10 +61,44 @@ export const SysJob = ObjectSchema.create({
group: 'Schedule',
}),

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

Expand Down
31 changes: 31 additions & 0 deletions packages/platform-objects/src/audit/sys-report-schedule.object.ts
Original file line number Diff line number Diff line change
Expand Up @@ -78,11 +78,42 @@ export const SysReportSchedule = ObjectSchema.create({
group: 'Schedule',
}),

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

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -20,8 +20,11 @@
* no default on either. The card's thesis is that every author invents this
* column differently, and the platform's own two precedents
* (`sys_job.timezone`: 100, no default; `sys_report_schedule.timezone`: 64,
* default `UTC`; neither validated) already disagree in three dimensions.
* The ruled pair must not become a third and a fourth spelling;
* default `UTC`) disagreed in three dimensions. #15872 closed the third by
* giving both the same `valueDomain` this pair carries — pinned next to
* them in `audit/platform-iana-timezone-columns.test.ts` — and left length
* and default alone. The ruled pair must not become a third and a fourth
* spelling;
* 3. the declared domain admits `UTC`, the fallback the contract names for a
* wholly unset chain, and the declared bound admits every zone the runtime
* enumerates. Why the first is not automatic — `Intl.supportedValuesOf`
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -167,8 +167,9 @@ export const SysBusinessUnit = ObjectSchema.create({
// pairs a bound with the `UTC` default; the enumeration's longest name on
// the repo's Node baseline is 30 characters and the tzdb caps each path
// component at 14, so 64 is twice the domain's real ceiling and the smaller
// of the two precedents (`sys_job.timezone` says 100, neither declares a
// domain — the residue card). No `defaultValue`, deliberately: an explicit
// of the two precedents (`sys_job.timezone` still says 100 — #15872 gave
// both precedents this same `valueDomain` and deliberately left that one
// bound unconverged). No `defaultValue`, deliberately: an explicit
// default here would mean "stop inheriting", which is the opposite of what
// an unset unit means.
timezone: Field.text({
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -252,9 +252,10 @@ export const SysOrganization = ObjectSchema.create({
// `sys_business_unit.timezone` by design (`text`, optional, `maxLength: 64`,
// `valueDomain: 'iana_time_zone'`, no default): the card's thesis is that
// every author invents this column differently, and the platform's own two
// precedents (`sys_job`, `sys_report_schedule`) already disagree on length,
// default and validation — the ruled pair is one spelling, pinned in
// `org-hierarchy-timezone.test.ts`.
// precedents (`sys_job`, `sys_report_schedule`) disagreed on length, default
// AND validation — #15872 closed the validation dimension by giving both
// this same `valueDomain`, and left the other two as each reader expects.
// The ruled pair is one spelling, pinned in `org-hierarchy-timezone.test.ts`.
timezone: Field.text({
label: 'Timezone',
required: false,
Expand Down
Loading