diff --git a/.changeset/report-schedule-timezone-guard.md b/.changeset/report-schedule-timezone-guard.md new file mode 100644 index 0000000000..7e9ce5a6e4 --- /dev/null +++ b/.changeset/report-schedule-timezone-guard.md @@ -0,0 +1,26 @@ +--- +"@objectstack/plugin-reports": minor +--- + +fix(plugin-reports)!: a non-member schedule `timezone` no longer discards the cron expression, and a schedule already holding one stops instead of firing on a cadence nobody asked for (#16291) + +**BREAKING** for a deployment that already stores a report schedule with a cron expression and a `timezone` that is not an IANA member. Such a schedule is delivering today, on the wrong cadence; after this change it does not deliver at all until a human corrects the zone. It ships as `minor` under the lockstep launch-window convention (`scripts/check-changeset-no-major.mjs` refuses `major`); the version number is not the signal here, this entry is. + + + +## What an upgrading operator has to do, and how to find out + +If `sys_report_schedule` holds a row whose `timezone` is not a real IANA zone **and** whose `cron_expression` is set, the sweep now marks it `last_status: 'failed'` with a `last_error` naming the zone, and stops running it. Correct the `timezone` on that row; the schedule resumes on the next sweep with no re-enable and no second action, because `active` and the past `next_run_at` are deliberately left alone. + +Only rows written **before** `valueDomain: 'iana_time_zone'` landed on that column can be in this state, and the set cannot grow: measured on a real kernel with a real SQLite driver, `insert` into `sys_report_schedule` with `timezone: 'Mars/Olympus'` is already refused today — `VALIDATION_FAILED · Timezone must be a valid IANA time zone identifier, e.g. Europe/Zurich (got "Mars/Olympus")`. A set that cannot grow is still not an empty one, which is why this carries a banner rather than a shrug. + +## The defect + +croner (10.0.1) answers a non-member zone in three different ways, and only the middle one was ever reached here: `new Cron(expr, { timezone })` **without a callback** validates the expression and lets any zone through, `nextRun()` on that instance then throws a `CronDate` conversion `TypeError`, and the callback form throws at construction. `scheduleReport`'s eager guard used the callback-less form, so the timezone half of its own input passed straight under a guard whose stated purpose was "a clear error at schedule time instead of a schedule that silently falls back to interval on sweep" — and `nextRunAt` caught that deferred throw and returned `from + interval_minutes`. A schedule authored as "every weekday 09:00 Asia/Shanghai" became "every 1440 minutes, forever", re-derived on every sweep, logged only as a complaint about a cron expression that was perfectly good. + +## What changed + +- **The create-time guard now asks the right question.** `scheduleReport` consults `isValueDomainMember('iana_time_zone', …)` from `@objectstack/spec/shared` — the same predicate `sys_report_schedule.timezone`'s `valueDomain` declaration enforces on write — and refuses a non-member with `VALIDATION_FAILED: invalid timezone '': not a member of the 'iana_time_zone' value domain`. One answer at both doors, so this one cannot accept what the storage door refuses; it says so earlier and names the input that is actually wrong. It applies whether or not a `cron_expression` is set, because the storage gate does too. **This is not what makes the change breaking:** the storage door already refuses the same value today, so no reachable accept set narrows — what moves is which door answers and how clearly. +- **The row now stores the string the scheduler evaluates.** An empty `timezone` was stored verbatim while every `new Cron` call site read it as `UTC`; it is normalised to `UTC` on the way in. +- **A schedule already holding an unusable zone is stopped, not rescheduled.** It is not run and its `next_run_at` is not advanced; `last_status` / `last_error` carry the reason. Repairing the value automatically was rejected: the intended zone is not recoverable from a typo, and rewriting it to `UTC` would deliver at yet another set of wrong instants while the row looked healthy. Interval-only schedules are untouched — interval arithmetic never consults the zone, so a legacy bad value there still delivers on the cadence its author asked for. +- **Both fall-back warnings name both inputs.** The "no next occurrence" and the former "invalid cron" lines each mentioned only the expression, so either of them on a timezone fault sent an investigator to audit the half that was fine. They now carry the expression *and* the zone, and the second no longer asserts the expression is the broken one. diff --git a/packages/platform-objects/src/audit/sys-report-schedule.object.ts b/packages/platform-objects/src/audit/sys-report-schedule.object.ts index 8ce0c68073..cb7114fa84 100644 --- a/packages/platform-objects/src/audit/sys-report-schedule.object.ts +++ b/packages/platform-objects/src/audit/sys-report-schedule.object.ts @@ -98,9 +98,18 @@ export const SysReportSchedule = ObjectSchema.create({ // `invalid cron ''` — 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. + // on. `scheduleReport`'s eager create-time guard did not catch it either; + // it constructed a callback-less `Cron` and so was blind to exactly this + // half of its own input. Refusing the write is what closes it HERE. + // + // [#16291] The reader's two halves are closed separately, and this line does + // not stand in for either: `scheduleReport` now consults + // `isValueDomainMember('iana_time_zone', …)` itself — this declaration's own + // predicate, so neither door can accept what the other refuses — and the + // sweep quarantines a row that was STORED before this line existed (it does + // not run it and does not advance `next_run_at`, and says so in + // `last_status` / `last_error`) rather than re-deriving a cadence from + // `interval_minutes` that nobody asked for. // // `maxLength: 64` and `defaultValue: 'UTC'` are BOTH unchanged. The bound is // already the value #14238 justified (twice the domain's real ceiling: the diff --git a/packages/plugins/plugin-reports/src/report-service.test.ts b/packages/plugins/plugin-reports/src/report-service.test.ts index b2f351bf8e..dc0f223894 100644 --- a/packages/plugins/plugin-reports/src/report-service.test.ts +++ b/packages/plugins/plugin-reports/src/report-service.test.ts @@ -465,6 +465,197 @@ describe('ReportService', () => { expect(engine._tables['sys_report_schedule'][0].last_status).toBe('ok'); }); + // ─── Schedule timezone (#16291) ───────────────────────────────── + // + // croner 10.0.1 has a THREE-state answer to a non-member IANA zone, and only + // the middle one was ever reached here (measured on Node v22.22.2): + // + // new Cron('0 9 * * *', { timezone: 'Mars/Olympus' }) -> constructs FINE + // .nextRun(from) -> TypeError: CronDate … + // new Cron('0 9 * * *', { timezone: 'Mars/Olympus' }, async () => {}) -> throws at construction + // + // So the create-time guard, which used the callback-less form, validated the + // expression and was blind to the zone; and `nextRunAt` caught the deferred + // throw and fell back to `interval_minutes` — turning "weekdays 09:00 + // Asia/Shanghai" into "every 1440 minutes, forever", logged as a complaint + // about a cron expression that was perfectly good. + describe('schedule timezone', () => { + const BAD_TZ = 'Mars/Olympus'; + + /** Store a schedule row directly — the shape a pre-#15872 row has. */ + function seedScheduleRow(reportId: string, patch: Record) { + const row = { + id: 'rsch_legacy', + report_id: reportId, + name: null, + interval_minutes: 1440, + cron_expression: null, + timezone: 'UTC', + active: true, + recipients: 'ops@t', + format: 'html_table', + subject_template: null, + owner_id: 'u1', + next_run_at: new Date(now.getTime() - 1000).toISOString(), + created_at: now.toISOString(), + updated_at: now.toISOString(), + ...patch, + }; + (engine._tables['sys_report_schedule'] ??= []).push(row); + return row; + } + + // ── The create-time door ── + + it('scheduleReport: refuses a non-member timezone instead of storing it', async () => { + const r = await svc.saveReport({ name: 'A', object: 'lead', query: {} }, CTX); + await expect(svc.scheduleReport({ + reportId: r.id, recipients: ['x@t'], cronExpression: '0 9 * * *', timezone: BAD_TZ, + }, CTX)).rejects.toThrow(/VALIDATION_FAILED/); + // Names the input that is actually wrong — not the cron expression, which + // is valid, and which the old guard was the only thing to mention. + await expect(svc.scheduleReport({ + reportId: r.id, recipients: ['x@t'], cronExpression: '0 9 * * *', timezone: BAD_TZ, + }, CTX)).rejects.toThrow(new RegExp(`timezone '${BAD_TZ}'`)); + expect(engine._tables['sys_report_schedule'] ?? []).toHaveLength(0); + }); + + it('scheduleReport: refuses a non-member timezone with no cron_expression too', async () => { + // One answer at both doors. `sys_report_schedule.timezone` carries + // `valueDomain: 'iana_time_zone'` (#15872), which refuses the value on + // WRITE whether or not a cron is set; a guard that accepted it here for + // interval schedules would hand the engine a row it is about to reject and + // report the divergence as a generic field error. + const r = await svc.saveReport({ name: 'A', object: 'lead', query: {} }, CTX); + await expect(svc.scheduleReport({ + reportId: r.id, recipients: ['x@t'], intervalMinutes: 60, timezone: BAD_TZ, + }, CTX)).rejects.toThrow(new RegExp(`VALIDATION_FAILED.*timezone '${BAD_TZ}'`)); + }); + + it('scheduleReport: the guard uses the shared predicate, so real zones still pass', async () => { + const r = await svc.saveReport({ name: 'A', object: 'lead', query: {} }, CTX); + for (const tz of ['UTC', 'Asia/Shanghai', 'America/New_York', 'Etc/GMT+8']) { + const s = await svc.scheduleReport({ + reportId: r.id, recipients: ['x@t'], cronExpression: '0 9 * * *', timezone: tz, + }, CTX); + expect(s.timezone).toBe(tz); + } + }); + + it('scheduleReport: stores the same zone string the scheduler evaluates', async () => { + // `''` is not an `iana_time_zone` member, but every `new Cron` call site + // reads it as UTC via `|| 'UTC'`. The row must not keep a value the storage + // gate refuses while the scheduler quietly treats it as something else. + const r = await svc.saveReport({ name: 'A', object: 'lead', query: {} }, CTX); + const s = await svc.scheduleReport({ + reportId: r.id, recipients: ['x@t'], cronExpression: '0 9 * * *', timezone: '', + }, CTX); + expect(s.timezone).toBe('UTC'); + expect(engine._tables['sys_report_schedule'][0].timezone).toBe('UTC'); + expect(s.next_run_at).toBe('2026-01-16T09:00:00.000Z'); + }); + + // ── The stored-row door: rows written before #15872 ── + + it('dispatchDue: a stored non-member timezone stops the schedule instead of rescheduling it', async () => { + const r = await svc.saveReport({ name: 'A', object: 'lead', query: {} }, CTX); + const seeded = seedScheduleRow(r.id, { + cron_expression: '0 9 * * 1-5', timezone: BAD_TZ, format: 'csv', + }); + + const result = await svc.dispatchDue(); + + expect(result).toEqual({ fired: 0, failed: 1, skipped: 0 }); + expect(email._sent).toHaveLength(0); + const stored = engine._tables['sys_report_schedule'][0]; + expect(stored.last_status).toBe('failed'); + expect(stored.last_error).toContain(BAD_TZ); + expect(stored.last_error).toContain('0 9 * * 1-5'); + // NOT advanced to `now + interval_minutes` — the whole defect was that it + // was, on every sweep, forever. + expect(stored.next_run_at).toBe(seeded.next_run_at); + expect(stored.next_run_at).not.toBe(new Date(now.getTime() + 1440 * 60_000).toISOString()); + }); + + it('dispatchDue: an interval-only schedule with a stored bad zone is left alone', async () => { + // The zone is load-bearing only for cron evaluation; interval arithmetic + // never consults it. Quarantining these would stop deliveries that are + // landing exactly when their author asked for them. + const r = await svc.saveReport({ name: 'A', object: 'lead', query: {} }, CTX); + seedScheduleRow(r.id, { cron_expression: null, interval_minutes: 60, timezone: BAD_TZ }); + + const result = await svc.dispatchDue(); + + expect(result.fired).toBe(1); + expect(email._sent).toHaveLength(1); + const stored = engine._tables['sys_report_schedule'][0]; + expect(stored.last_status).toBe('ok'); + expect(stored.next_run_at).toBe(new Date(now.getTime() + 60 * 60_000).toISOString()); + }); + + it('dispatchDue: correcting the stored zone resumes the schedule with no other action', async () => { + // Why the quarantine leaves `active` set and `next_run_at` in the past: + // the row stays due, so the sweep picks it up again by itself. + const r = await svc.saveReport({ name: 'A', object: 'lead', query: {} }, CTX); + seedScheduleRow(r.id, { cron_expression: '0 9 * * *', timezone: BAD_TZ, format: 'csv' }); + + expect((await svc.dispatchDue()).failed).toBe(1); + engine._tables['sys_report_schedule'][0].timezone = 'Asia/Shanghai'; + + const result = await svc.dispatchDue(); + expect(result.fired).toBe(1); + expect(email._sent).toHaveLength(1); + const stored = engine._tables['sys_report_schedule'][0]; + expect(stored.last_status).toBe('ok'); + // 09:00 Asia/Shanghai (UTC+8) on the 16th = 01:00Z — the instant its author + // actually asked for, not `now + 1440m`. + expect(stored.next_run_at).toBe('2026-01-16T01:00:00.000Z'); + }); + + // ── The warning text: both paths, neither pointing at the wrong input ── + + it('nextRunAt: the no-occurrence warning names the timezone as well as the cron', async () => { + const warn = vi.fn(); + const logged = new ReportService({ + engine: engine as any, email, clock: { now: () => now }, logger: { warn }, + resolveOwnerContext: async (id: string) => ({ userId: id, positions: [], permissions: [] }), + }); + const r = await logged.saveReport({ name: 'A', object: 'lead', query: {} }, CTX); + // 30 February never occurs; croner returns null rather than throwing. + await logged.scheduleReport({ + reportId: r.id, recipients: ['x@t'], cronExpression: '0 0 30 2 *', timezone: 'Asia/Shanghai', + }, CTX); + + const line = warn.mock.calls.map(c => String(c[0])).find(m => m.includes('no next occurrence')); + expect(line).toBeDefined(); + expect(line).toContain("timezone 'Asia/Shanghai'"); + expect(line).toContain("cron '0 0 30 2 *'"); + }); + + it('nextRunAt: the un-evaluatable warning names the timezone and stops calling the cron invalid', async () => { + const warn = vi.fn(); + const logged = new ReportService({ + engine: engine as any, email, clock: { now: () => now }, logger: { warn }, + resolveOwnerContext: async (id: string) => ({ userId: id, positions: [], permissions: [] }), + }); + const r = await logged.saveReport({ name: 'A', object: 'lead', query: {} }, CTX); + // A row whose cron the create-time guard would have refused — the shape + // that reaches `nextRunAt` through `advanceSchedule` on a sweep. + seedScheduleRow(r.id, { cron_expression: 'not a cron', timezone: 'Asia/Shanghai' }); + + await logged.dispatchDue(); + + const line = warn.mock.calls.map(c => String(c[0])).find(m => m.includes('could not be evaluated')); + expect(line).toBeDefined(); + expect(line).toContain("timezone 'Asia/Shanghai'"); + expect(line).toContain("cron 'not a cron'"); + // The old text asserted the expression was the broken half. On a timezone + // fault that accusation was simply false, and it is the reason this card + // treats the warning as part of the defect rather than as cosmetics. + expect(warn.mock.calls.map(c => String(c[0])).join('\n')).not.toContain('invalid cron'); + }); + }); + // ─── Authorization (#2980) ────────────────────────────────────── describe('access control', () => { const OTHER = { userId: 'u2', tenantId: 't1', positions: [], permissions: [] }; diff --git a/packages/plugins/plugin-reports/src/report-service.ts b/packages/plugins/plugin-reports/src/report-service.ts index ed400dec60..d11eff8969 100644 --- a/packages/plugins/plugin-reports/src/report-service.ts +++ b/packages/plugins/plugin-reports/src/report-service.ts @@ -18,6 +18,12 @@ import type { // `OwnerContextResolver`; naming the retired six-field shape here made this // file's own type say it could not see what that resolver returns. import type { ExecutionContext } from '@objectstack/spec/kernel'; +// [#16291] THE membership predicate for `iana_time_zone` — the same one +// `sys_report_schedule.timezone`'s `valueDomain: 'iana_time_zone'` write gate +// consults (#15872), and the same one the settings door uses. Imported rather +// than re-derived on purpose: a second hand-written time-zone judgement in this +// package would be a second answer, and the two doors would drift. +import { isValueDomainMember } from '@objectstack/spec/shared'; import { Cron } from 'croner'; /** @@ -57,6 +63,32 @@ const DEFAULT_FORMAT: ReportFormat = 'csv'; const DEFAULT_INTERVAL_MIN = 1440; const DEFAULT_LIMIT = 1000; +/** The default this file documents in four places, in one spelling. */ +const DEFAULT_TIMEZONE = 'UTC'; + +/** + * The zone croner is actually handed for a schedule — `timezone || 'UTC'`, the + * exact expression every `new Cron(...)` call site in this file uses. Membership + * is judged on THIS string, never on the raw column value: a check that judges a + * different string than the scheduler receives is a phantom check. + */ +function effectiveTimezone(timezone?: string | null): string { + return timezone || DEFAULT_TIMEZONE; +} + +/** + * [#16291] Is this schedule's zone one croner can resolve? + * + * croner 10.0.1 does NOT answer this at construction: `new Cron(expr, { timezone })` + * WITHOUT a callback validates the expression and nothing else, so a non-member + * zone constructs fine and only throws later, from `nextRun()`. That is why the + * create-time guard could not see the timezone half of its own input, and why + * asking the constructor harder is not the fix — asking the right question is. + */ +function isUsableTimezone(timezone?: string | null): boolean { + return isValueDomainMember('iana_time_zone', effectiveTimezone(timezone)); +} + function uid(prefix: string): string { const g: any = globalThis as any; if (g.crypto?.randomUUID) return `${prefix}_${g.crypto.randomUUID()}`; @@ -602,17 +634,40 @@ export class ReportService implements IReportService { const now = this.clock.now(); const interval = input.intervalMinutes ?? DEFAULT_INTERVAL_MIN; const cron = input.cronExpression?.trim() || null; + // [#16291] Validate the TIMEZONE explicitly, and first. The guard below has + // always stated its purpose as "a clear error at schedule time instead of a + // schedule that silently falls back to interval on sweep" — but it asked the + // callback-less `Cron` constructor, which validates the expression and lets + // any string through as the zone (croner defers that to `nextRun()`). So the + // half of the input the guard could not see produced VERBATIM the outcome the + // guard exists to prevent. + // + // The predicate is the one `sys_report_schedule.timezone` already enforces on + // write (#15872), applied to the string croner is actually handed, so this + // door can never accept a value the storage door refuses — it only says so + // earlier, and in words that name the input that is wrong. + const timezone = effectiveTimezone(input.timezone); + if (!isUsableTimezone(timezone)) { + throw new Error( + `VALIDATION_FAILED: invalid timezone '${timezone}': not a member of the 'iana_time_zone' value domain`, + ); + } if (cron) { // 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' }); + new Cron(cron, { timezone }); } catch (err) { throw new Error(`VALIDATION_FAILED: invalid cron_expression '${cron}': ${(err as Error).message}`); } } + // The row stores, and the scheduler evaluates, the SAME string the guard just + // judged. `?? 'UTC'` here used to leave an empty-string `timezone` on the row + // while `nextRunAt`'s `|| 'UTC'` scheduled it in UTC — a row whose stored zone + // the storage gate would refuse and whose scheduler behaviour disagreed with + // it. `effectiveTimezone` is that one spelling. const nextRun = this.nextRunAt( - { cron_expression: cron, interval_minutes: interval, timezone: input.timezone ?? 'UTC' }, + { cron_expression: cron, interval_minutes: interval, timezone }, now, ).toISOString(); const id = uid('rsch'); @@ -622,7 +677,7 @@ export class ReportService implements IReportService { name: input.name ?? null, interval_minutes: interval, cron_expression: cron, - timezone: input.timezone ?? 'UTC', + timezone, active: input.active !== false, recipients: input.recipients.join(','), format: input.format ?? 'html_table', @@ -708,6 +763,49 @@ export class ReportService implements IReportService { let fired = 0, failed = 0, skipped = 0; for (const schedule of list) { try { + // [#16291] A stored zone croner cannot resolve is a CONFIGURATION fault, + // not a run failure, and it is the one case where continuing is worse + // than stopping: `nextRunAt` would discard the cron expression and + // re-derive `next_run_at` from `interval_minutes`, so "every weekday + // 09:00 Asia/Shanghai" becomes "every 1440 minutes, forever" — the wrong + // instants, indefinitely, rediscovered on every sweep. + // + // #15872 refuses such a value on the WRITE path, and `scheduleReport` now + // refuses it at the service door, but neither heals a row already stored: + // `valueDomain` is in the written-values-only transition-gate class, so + // rows that predate it are never re-validated. This arm is the only place + // those rows are seen at all, which is why the answer for them lives here. + // + // The answer is MARK AND STOP, not repair: the intended zone is not + // recoverable from a typo, and rewriting it to UTC would silently deliver + // at yet another set of wrong instants while looking healthy. So the + // schedule does not run, `next_run_at` is NOT advanced, and the row itself + // carries the reason. Leaving `active` alone and `next_run_at` in the past + // is deliberate: it is the same posture this loop already takes for a + // schedule whose report has vanished, and it means the sweep resumes the + // schedule by itself the moment an admin corrects the zone — no second + // action, no re-enable. + // + // Guarded on `cron_expression` because that is exactly where the zone is + // load-bearing: interval arithmetic never consults it, so an interval-only + // schedule carrying a legacy bad zone still delivers on the cadence its + // author asked for and is left alone. + const scheduleCron = (schedule.cron_expression ?? '').trim(); + if (scheduleCron && !isUsableTimezone(schedule.timezone)) { + const badZone = effectiveTimezone(schedule.timezone); + failed++; + await this.markSchedule(schedule.id, { + last_status: 'failed', + last_error: + `timezone '${badZone}' is not a valid IANA time zone, so cron '${scheduleCron}' cannot be evaluated; ` + + 'the schedule is not being run and next_run_at is not being advanced — correct the timezone to resume', + }); + this.logger?.warn?.( + `ReportService.dispatchDue: schedule ${schedule.id} has unusable timezone '${badZone}'; not run, not rescheduled`, + ); + continue; + } + const row = await this.loadReportRow(schedule.report_id); if (!row) { skipped++; @@ -804,23 +902,38 @@ export class ReportService implements IReportService { * `interval_minutes` (the documented `sys_report_schedule` contract) and is * evaluated in the schedule's `timezone` (default UTC) via croner — the same * library the job scheduler uses. Falls back to `from + interval_minutes` for - * interval schedules, and also if a cron expression is invalid or has no - * future occurrence (logged; never throws into the sweep). `from` is the - * reference instant (the injected clock), so `today()`-style boundaries honor - * the test clock. + * interval schedules, and also if a cron expression cannot be evaluated or has + * no future occurrence — both logged naming the expression AND the zone, since + * either can be the cause; never throws into the sweep. `from` is the reference + * instant (the injected clock), so `today()`-style boundaries honor the test + * clock. + * + * ⚠️ This stays a PURE function of its arguments — it computes an instant, it + * does not decide policy. The sweep's answer to an unusable zone (do not run, + * do not advance, mark the row) lives in `dispatchDue`, where the row and the + * engine are in hand; the fallback here survives only so a caller that has + * already passed that arm cannot throw into the loop. */ private nextRunAt( schedule: { cron_expression?: string | null; interval_minutes?: number | null; timezone?: string | null }, from: Date, ): Date { const cron = (schedule.cron_expression ?? '').trim(); + const timezone = effectiveTimezone(schedule.timezone); if (cron) { + // [#16291] BOTH warnings name BOTH halves of what croner was handed, and + // neither accuses either half any more. A fall back to interval is + // attributable to the expression OR the zone — croner reports an unusable + // zone as a `CronDate` conversion TypeError out of `nextRun()`, which the + // old text relabelled `invalid cron ''` — and an investigator sent to + // audit a cron expression that was perfectly good is worse off than one + // told nothing. Say what failed, name both inputs, blame neither. try { - const next = new Cron(cron, { timezone: schedule.timezone || 'UTC' }).nextRun(from); + const next = new Cron(cron, { timezone }).nextRun(from); if (next) return next; - this.logger?.warn?.(`ReportService: cron '${cron}' has no next occurrence; falling back to interval`); + this.logger?.warn?.(`ReportService: cron '${cron}' (timezone '${timezone}') has no next occurrence; falling back to interval`); } catch (err) { - this.logger?.warn?.(`ReportService: invalid cron '${cron}'; falling back to interval`, err); + this.logger?.warn?.(`ReportService: cron '${cron}' (timezone '${timezone}') could not be evaluated; falling back to interval`, err); } } const interval = schedule.interval_minutes ?? DEFAULT_INTERVAL_MIN; diff --git a/packages/plugins/plugin-reports/vitest.config.ts b/packages/plugins/plugin-reports/vitest.config.ts index 5e0591efc4..6baaa4a5d0 100644 --- a/packages/plugins/plugin-reports/vitest.config.ts +++ b/packages/plugins/plugin-reports/vitest.config.ts @@ -1,12 +1,44 @@ // Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license. -// This config exists for exactly one setting; everything else stays on -// vitest's defaults, deliberately — a key added here re-specifies behaviour +// The `test:` block here carries exactly one setting; everything else stays on +// vitest's defaults, deliberately — a key added there re-specifies behaviour // for every test file in the package (packages/cli/vitest.config.ts's header -// records the incident that taught that). +// records the incident that taught that). `resolve.alias` below is not such a +// key: it changes which BYTES a specifier resolves to, not how any test runs. import { defineConfig } from 'vitest/config'; +import path from 'node:path'; export default defineConfig({ + resolve: { + // [#16291] `report-service.ts` imports `isValueDomainMember` from + // `@objectstack/spec/shared` as a VALUE (the type-only imports beside it are + // erased and never resolve at run time, which is why this package needed no + // alias table until now). Unaliased, a value specifier resolves through the + // workspace link to `@objectstack/spec`'s `dist/` — a BUILD ARTIFACT — so the + // timezone tests below would be a verdict about build state rather than about + // the predicate in this checkout, and a dist merely BEHIND would let them pass + // against the old membership answer with nothing in the output saying so + // (`pnpm check:test-source-alias`). + // + // ONE anchored rule for every published `@objectstack/spec` namespace rather + // than an entry for the one subpath reached today: spec's export map is + // UNIFORM (every namespace is `src//index.ts`, with no FILE-shaped + // subpath), so the rule cannot go stale as tests reach new namespaces. Same + // shape as packages/metadata, packages/runtime and packages/qa/downstream-contract. + // + // ⛔ Array form with ANCHORED patterns, never the object form: object keys + // match by PREFIX, so a bare `@objectstack/spec` key with a FILE replacement + // swallows `@objectstack/spec/shared` and resolves it to + // `…/spec/src/index.ts/shared` — ENOTDIR at run time, from a config that + // reads as correct. + alias: [ + { + find: /^@objectstack\/spec\/([a-z-]+)$/, + replacement: path.join(path.resolve(__dirname, '../..'), 'spec/src/$1/index.ts'), + }, + { find: /^@objectstack\/spec$/, replacement: path.resolve(__dirname, '../../spec/src/index.ts') }, + ], + }, test: { // A late console.* must not redden a green suite (#10374): vitest's worker // forwards console output over RPC and discards the promise, and a write