From d79440c2625b802a19773e9115c009b78e45897b Mon Sep 17 00:00:00 2001 From: Warren Date: Tue, 1 Sep 2026 10:21:06 +0000 Subject: [PATCH] Refuse a fractional or absurd cadence number at write time, not at dispatch MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `due_offset_days`, `lead_days` and `grace_days` on `duly_duty` (and their `duly_catalog_item` templates) declared no numeric constraints, and the engine's number validator enforces `min` / `max` / `scale` only when they are declared. So `due_offset_days: 1.5` saved clean, passed `pnpm validate` and rendered fine — then threw days later inside the nightly batch, recorded as `invalid_cadence` against the JOB rather than against the duty holding it. Declarative, not scripted (AGENTS.md rule 9): `scale: 0` plus bounds. The platform's refusal was measured before deciding a hand-written rule was needed, and it already names the field by its label, the limit and what arrived — "Offset (days, 0 = anchor day) must have at most 0 decimal places (got 1)" — so nothing is layered on top of it. The three fields do NOT share a failure mode, measured one at a time: due_offset_days: 1.5 `dueDateFor` throws; run `degraded`, no tasks lead_days: 2.5 throws one function over, through `addCalendarDays`, which negates — the message names -2.5, not 2.5 grace_days: 2.5 throws nowhere. Its only evaluating reader is the overdue escalation's CEL gate, which wraps it in `int()`: `int(2.5) == 2`, so it escalates on the day a grace of 2 would, silently, forever. `grace_days` stops at 14 rather than 366 because the overdue sweep looks back 15 days and fires on `due_date + grace_days + 1`: anything above 14 is a value the product silently cannot honour. The demo catalog shipped an item with 21, which had never once escalated; it is now 14. Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_01SqkTcrxUFci7nqXdbBSe2p --- src/data/demo-catalog.ts | 8 +- src/flows/reminders.flow.ts | 19 +- src/objects/catalog-item.object.ts | 24 +- src/objects/duty.object.ts | 77 ++++++- test/cadence-number-constraints.test.ts | 292 ++++++++++++++++++++++++ test/reminders.test.ts | 24 +- 6 files changed, 423 insertions(+), 21 deletions(-) create mode 100644 test/cadence-number-constraints.test.ts diff --git a/src/data/demo-catalog.ts b/src/data/demo-catalog.ts index 4de18fc..fbf13a1 100644 --- a/src/data/demo-catalog.ts +++ b/src/data/demo-catalog.ts @@ -203,7 +203,13 @@ export const CATALOG_ITEMS: readonly DemoCatalogItem[] = [ // Same reasoning as the semi-annual audit above — a year's notice for a // year's obligation, which is what lets it stagnate long before it is late. leadDays: 120, - graceDays: 21, + // Was 21, and 21 never worked: the overdue escalation fires on + // `due_date + grace_days + 1` and its sweep looks back 15 days, so day one + // for this item fell outside the window and it was never escalated — a + // live instance, in our own demo data, of the silent late-failure #82 is + // about. `duly_catalog_item.grace_days` now stops at 14 (the largest the + // sweep can honour) and this row would be refused at seed time. + graceDays: 14, description: 'Re-run the site induction for every contractor still holding a pass, and retire the passes nobody claimed.', reference: 'Site Safety Standard SS-15 §3', }, diff --git a/src/flows/reminders.flow.ts b/src/flows/reminders.flow.ts index 9c9568e..9842ba0 100644 --- a/src/flows/reminders.flow.ts +++ b/src/flows/reminders.flow.ts @@ -177,13 +177,18 @@ const DUTY_GATE_FIELDS = ['id', 'grace_days', 'effective_from', 'effective_to'] * How far back the overdue sweep looks, and therefore the largest * `grace_days` the day-one escalation can honour. * - * `duly_duty.grace_days` declares `min: 0` and NO maximum, so these two - * numbers are coupled with nothing to hold them together but this comment and - * the test that pins it: a duty with grace ≥ 15 has its escalation day fall - * outside the swept window and is never escalated. Raising the field's ceiling - * means raising this. The alternative — an unbounded lookback — re-launches - * the flow every day for every task ever missed, which is the "ancient record - * re-alerting forever" the trigger's own `withinDays` doc warns about. + * A duty with grace ≥ 15 has its escalation day fall outside the swept window + * and is never escalated — silently. That used to be reachable: `grace_days` + * declared `min: 0` and no maximum, and the demo catalog shipped an item with + * 21. Since #82 the field declares `max: 14`, which is THIS number minus one, + * so every value the object accepts is a value this sweep can honour. + * + * The two are still two numbers in two files, coupled by nothing but this + * comment and `test/reminders.test.ts`, which reads both and refuses to let + * either move alone. Raising the field's ceiling means raising this. The + * alternative — an unbounded lookback — re-launches the flow every day for + * every task ever missed, which is the "ancient record re-alerting forever" + * the trigger's own `withinDays` doc warns about. */ const OVERDUE_LOOKBACK_DAYS = 15; diff --git a/src/objects/catalog-item.object.ts b/src/objects/catalog-item.object.ts index 26357ca..27d9f0a 100644 --- a/src/objects/catalog-item.object.ts +++ b/src/objects/catalog-item.object.ts @@ -78,22 +78,40 @@ export const CatalogItem = ObjectSchema.create({ description: 'Anchors the due date inside a period. Only a recurring duty has one; blank (and forbidden) for standing and one-off.', }), + // `scale` and the bounds are `duly_duty`'s, to the digit — see the long + // block on that object for what each one was measured to prevent (#82). + // They belong HERE as well as there because this object is where the + // values are AUTHORED: `applyCatalogHandler` copies all three onto every + // duty it creates, through `engine.insert`, which validates. A catalog + // item carrying `due_offset_days: 1.5` is therefore not a quiet + // inconsistency — it is an apply that refuses partway through, having + // already created duties for the first N people, and the refusal names + // `duly_duty` rather than the catalog item the value actually lives on. + // Stopping it at the source is the difference between one loud refusal on + // the row being edited and a half-finished fan-out. due_offset_days: Field.number({ label: 'Offset (days, 0 = anchor day)', defaultValue: F`record.form != "recurring" ? null : 0`, - description: 'Days from the anchor day, which is offset 0. On "Start of period": 0 = the first day of the period, 4 = the fifth day. On "End of period": 0 = the last day of the period, -3 = three days before the last. Only a recurring duty has a period to offset into; blank (and forbidden) for standing and one-off.', + scale: 0, + min: -366, + max: 366, + description: 'Days from the anchor day, which is offset 0. On "Start of period": 0 = the first day of the period, 4 = the fifth day. On "End of period": 0 = the last day of the period, -3 = three days before the last. Whole days, and within a year either side of the anchor. Only a recurring duty has a period to offset into; blank (and forbidden) for standing and one-off.', }), lead_days: Field.number({ label: 'Lead time (days)', defaultValue: F`record.form != "recurring" ? null : 7`, + scale: 0, min: 0, - description: 'Only a recurring duty is dispatched with a lead window; blank (and forbidden) for standing and one-off.', + max: 366, + description: 'Whole days, up to a year. Only a recurring duty is dispatched with a lead window; blank (and forbidden) for standing and one-off.', }), grace_days: Field.number({ label: 'Grace (days)', defaultValue: F`record.form == "standing" ? null : 0`, + scale: 0, min: 0, - description: 'Meaningless for a standing duty, which never has a task; still applies to a one-off\'s.', + max: 14, + description: 'Whole days, up to 14 — the overdue reminder sweeps 15 days back, so a longer grace would never fire. Meaningless for a standing duty, which never has a task; still applies to a one-off\'s.', }), regulation_ref: Field.text({ diff --git a/src/objects/duty.object.ts b/src/objects/duty.object.ts index c0bbdaf..7cc430b 100644 --- a/src/objects/duty.object.ts +++ b/src/objects/duty.object.ts @@ -155,24 +155,95 @@ export const Duty = ObjectSchema.create({ description: 'Anchors the due date inside a period. Only a recurring duty has one; blank (and forbidden) for standing and one-off.', }), + // ── Why these three carry `scale` and bounds (#82) ──────────────────── + // All three are WHOLE DAYS, and the engine's number validator enforces + // `min` / `max` / `scale` only when they are DECLARED. Undeclared, a + // fractional or absurd value saves clean, passes `pnpm validate` and + // renders fine — then fails days later inside the nightly batch, recorded + // against the JOB rather than against the duty that holds the bad value. + // + // `scale: 0` rather than a sixth hand-written validation: it is the + // platform's own declarative answer to exactly this (AGENTS.md rule 9), + // and it does something no rule can — the form renders a whole-number + // input, so the value is harder to type wrong in the first place. The + // platform's refusal was measured before this was decided rather than + // assumed insufficient (the card left "or a script validation with a + // product-voice message" open), and it turns out to name the field by its + // LABEL, the limit, and what arrived: + // + // ValidationError: Offset (days, 0 = anchor day) must have at most 0 + // decimal places (got 1) code VALIDATION_FAILED + // fields[0]: { field: 'due_offset_days', code: 'max_scale', + // constraint: { scale: 0, actual: 1 } } + // + // That is already product voice, so nothing is layered on top of it. The + // bounds read the same way — "Grace (days) must be ≤ 14". + // + // ── The three do NOT share a failure mode ───────────────────────────── + // Same declaration, three different routes to the period engine — measured + // one duty at a time on 17.2.0, against the real engine and the real CEL + // evaluator, because "same declaration" is not "same behaviour": + // + // due_offset_days: 1.5 `dueDateFor` throws, `planForDuty` catches it as + // `invalid_cadence`, the run reports `degraded` + // and the duty produces no tasks — + // "dueOffsetDays must be a whole number of days, + // received 1.5". + // lead_days: 2.5 Throws too, but one function over: + // `visibleFromFor`, reached through + // `addCalendarDays`, which NEGATES its argument. + // So the message names a number nobody typed — + // "leadDays must be a whole number of days, + // received -2.5". + // grace_days: 2.5 Throws NOWHERE. It never reaches the period + // engine at all; its only evaluating reader is the + // overdue escalation's CEL gate, which wraps it in + // `int()`. Measured: `int(2.5) == 2`, + // `int(2.9) == 2`. A duty declaring 2.5 days of + // grace escalates on precisely the day one + // declaring 2 does — silently, forever. + // + // ── The bounds ──────────────────────────────────────────────────────── + // `9e9` was accepted here too, and failed later still: measured as + // "dueDate must be a YYYY-MM-DD calendar date, received 0NaN-NaN-NaN", + // which names neither the field nor anything the author typed. An anchor + // or a lead window reaching more than a year outside its own period is a + // typo, not a schedule, so ±366 / 0..366 is where they stop. + // + // `grace_days` stops at 14, and that number is NOT a taste call. The + // overdue escalation fires on `due_date + grace_days + 1` and its sweep + // looks back `OVERDUE_LOOKBACK_DAYS` (15) days + // (`src/flows/reminders.flow.ts`), so a grace of 15 or more puts day one + // outside the swept window and the escalation NEVER FIRES — the same + // silent inertness this card is about, one flow over. 14 is the largest + // grace the product can currently honour; raising it means raising the + // lookback, and `test/reminders.test.ts` holds the two numbers together so + // neither can move alone. due_offset_days: Field.number({ label: 'Offset (days, 0 = anchor day)', defaultValue: F`record.form != "recurring" ? null : 0`, - description: 'Days from the anchor day, which is offset 0. On "Start of period": 0 = the first day of the period, 4 = the fifth day. On "End of period": 0 = the last day of the period, -3 = three days before the last. Only a recurring duty has a period to offset into; blank (and forbidden) for standing and one-off.', + scale: 0, + min: -366, + max: 366, + description: 'Days from the anchor day, which is offset 0. On "Start of period": 0 = the first day of the period, 4 = the fifth day. On "End of period": 0 = the last day of the period, -3 = three days before the last. Whole days, and within a year either side of the anchor — anything larger is a typo, not a schedule. Only a recurring duty has a period to offset into; blank (and forbidden) for standing and one-off.', }), lead_days: Field.number({ label: 'Lead time (days)', defaultValue: F`record.form != "recurring" ? null : 7`, + scale: 0, min: 0, - description: 'How far ahead of the due date the task appears in the owner\'s list. A task that shows up on its due date is a task that is already late. Only a recurring duty is dispatched with a lead window; blank (and forbidden) for standing and one-off.', + max: 366, + description: 'How far ahead of the due date the task appears in the owner\'s list. A task that shows up on its due date is a task that is already late. Whole days, up to a year. Only a recurring duty is dispatched with a lead window; blank (and forbidden) for standing and one-off.', }), grace_days: Field.number({ label: 'Grace (days)', defaultValue: F`record.form == "standing" ? null : 0`, + scale: 0, min: 0, - description: 'Days after the due date before an open task counts as late. Meaningless for a standing duty, which never has a task; still applies to a one-off\'s.', + max: 14, + description: 'Days after the due date before an open task counts as late. Whole days, up to 14 — the overdue reminder sweeps 15 days back, so a longer grace would put day one outside the window and never fire at all. Meaningless for a standing duty, which never has a task; still applies to a one-off\'s.', }), // A global product cannot compute "the 5th of the month" without knowing diff --git a/test/cadence-number-constraints.test.ts b/test/cadence-number-constraints.test.ts new file mode 100644 index 0000000..a530a55 --- /dev/null +++ b/test/cadence-number-constraints.test.ts @@ -0,0 +1,292 @@ +// Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license. + +import { afterAll, beforeAll, describe, expect, it } from 'vitest'; +import { AppPlugin, ObjectKernel, createStandaloneStack } from '@objectstack/runtime'; + +import stack from '../objectstack.config.js'; +import { CatalogItem, Duty } from '../src/objects/index.js'; +import { FREQUENCIES } from '../src/functions/period.js'; +import { planDispatch, type DispatchDuty } from '../src/jobs/dispatch.plan.js'; + +/** + * #82 — `due_offset_days`, `lead_days` and `grace_days` accepted values the + * product cannot carry out. + * + * The defect is the shape #24 had one field over: a value that validates + * clean on the record and fails days later, inside the nightly batch, + * attributed to the job rather than to the duty holding it. The fix is + * DECLARATIVE — `scale` / `min` / `max` on the field (AGENTS.md rule 9) — + * because the engine's number validator enforces those three and only when + * they are declared, and because a declared `scale: 0` also makes the form + * render a whole-number input. + * + * ── The three fields did NOT behave the same, and that is the point ────── + * They share a declaration and reach the period engine by different routes. + * Measured on 17.2.0 before the fix, one duty at a time: + * + * due_offset_days: 1.5 `dueDateFor` throws; the run is `degraded` and the + * duty produces nothing — + * "dueOffsetDays must be a whole number of days, + * received 1.5" + * lead_days: 2.5 throws one function over (`visibleFromFor`, reached + * through `addCalendarDays`, which NEGATES), so the + * message names a value nobody typed — + * "leadDays must be a whole number of days, + * received -2.5" + * grace_days: 2.5 throws nowhere, ever. It never reaches the period + * engine; its only evaluating reader is the overdue + * escalation's CEL gate, which wraps it in `int()`. + * `int(2.5) == 2`, so it escalates on the day a + * grace of 2 would — silently, forever. + * + * The suite below therefore proves the WRITE is refused (one place, one + * mechanism), and separately that every value the declaration still admits is + * one the consumers can actually carry out. + * + * ── Why this boots a real engine ───────────────────────────────────────── + * A structural assertion (`Duty.fields.x.scale === 0`) proves the key is + * present, not that anything enforces it. Only a real insert proves the + * refusal, and only a real UPDATE proves the update path — the two are + * different code paths in the engine, and #65 was exactly a rule that held on + * insert and did nothing on update. + */ + +type AnyRow = Record; + +interface FieldFault { + field: string; + code: string; + message: string; + label?: string; + constraint?: Record; +} + +let kernel: { getService(name: string): unknown; shutdown?(): Promise } | undefined; +let data: { + insert(o: string, d: AnyRow, x?: AnyRow): Promise; + update(o: string, d: AnyRow, x?: AnyRow): Promise; +}; + +beforeAll(async () => { + const { plugins } = await createStandaloneStack({ + databaseDriver: 'memory', + skipSeedData: true, + // Must not resolve to a real path — see cadence-conditional-defaults.test.ts: + // a local `pnpm build` would make the suite report on the last BUILD. + artifactPath: 'dist/objectstack.this-suite-must-not-load-an-artifact.json', + }); + const k = new ObjectKernel(); + for (const plugin of plugins) await k.use(plugin); + await k.use(new AppPlugin(stack, undefined, { skipSeedData: true })); + await k.bootstrap(); + kernel = k as unknown as typeof kernel; + data = k.getService('data') as typeof data; +}, 180_000); + +afterAll(async () => { + await kernel?.shutdown?.(); +}); + +/** + * Assert a refusal by its ENVELOPE (ADR-0112), never by the bare fact that it + * threw. `expect(...).rejects.toThrow()` would pass on ANY error — including + * the "Owner is required" a malformed fixture produces — which is how a + * rejection test ends up green against a field that enforces nothing. + * + * The engine-level envelope carries `code` and a per-field `fields[]`; it + * carries no HTTP `status` (measured: `undefined` — status is the REST + * layer's mapping, not this one's), so the field-level `code` is what stands + * in for it here and it is the more precise assertion anyway: + * `max_scale` and `max_value` are different refusals. + */ +async function refusal(promise: Promise): Promise<{ code: unknown; message: string; fields: FieldFault[] }> { + try { + await promise; + } catch (error: any) { + return { + code: error?.code, + message: String(error?.message ?? ''), + fields: (error?.fields ?? []) as FieldFault[], + }; + } + throw new Error('expected the write to be refused, but it resolved'); +} + +let seq = 0; + +const dutyRow = (over: AnyRow): AnyRow => ({ + name: `Duty ${++seq}`, + owner: `user_${seq}`, + source: 'self', + status: 'active', + form: 'recurring', + frequency: 'monthly', + ...over, +}); + +const catalogRow = (over: AnyRow): AnyRow => ({ + name: `Item ${++seq}`, + position_code: 'test_position', + form: 'recurring', + frequency: 'monthly', + ...over, +}); + +const insertDuty = (over: AnyRow) => data.insert('duly_duty', dutyRow(over)); +const insertCatalogItem = (over: AnyRow) => data.insert('duly_catalog_item', catalogRow(over)); + +/** The declared box, as one table both objects are checked against. */ +const BOUNDS = { + due_offset_days: { scale: 0, min: -366, max: 366 }, + lead_days: { scale: 0, min: 0, max: 366 }, + grace_days: { scale: 0, min: 0, max: 14 }, +} as const; + +type CadenceField = keyof typeof BOUNDS; +const CADENCE_FIELDS = Object.keys(BOUNDS) as CadenceField[]; + +// ───────────────────────────────────────────────────────────────────────── +// The declaration +// ───────────────────────────────────────────────────────────────────────── + +describe('the constraints are declared, not scripted', () => { + it.each(CADENCE_FIELDS)('duly_duty.%s declares scale, min and max', (field) => { + expect(Duty.fields[field]).toMatchObject(BOUNDS[field]); + }); + + it.each(CADENCE_FIELDS)('duly_catalog_item.%s declares the SAME box', (field) => { + // A catalog item is a duty template and `applyCatalogHandler` copies all + // three onto every duty it creates. A laxer bound here is the same silent + // hole reached one object earlier — and a STRICTER one would refuse a + // template for a duty that is perfectly legal. + expect(CatalogItem.fields[field]).toMatchObject(BOUNDS[field]); + }); + + it('no hand-written validation was added for any of it', () => { + // The card left this open — `scale: 0` versus a `script` validation with a + // product-voice message — and the answer is the platform's own key + // (AGENTS.md rule 9). The measured refusal is already product voice + // ("Offset (days, 0 = anchor day) must have at most 0 decimal places + // (got 1)"), so there is nothing layered on top of it. This pins that: + // a rule re-stating a declared bound is two sources of truth for one + // limit, and the one that drifts is always the hand-written one. + for (const object of [Duty, CatalogItem]) { + for (const rule of object.validations ?? []) { + const source = JSON.stringify(rule); + for (const needle of ['decimal', 'whole number', '366', 'scale']) { + expect(source, `validation '${rule.name}' on ${object.name}`).not.toContain(needle); + } + } + } + }); +}); + +// ───────────────────────────────────────────────────────────────────────── +// The refusals — a real engine, a real write +// ───────────────────────────────────────────────────────────────────────── + +describe('a fractional cadence value is refused at write time', () => { + it.each(CADENCE_FIELDS)('duly_duty.%s: 1.5 is refused as max_scale', async (field) => { + const { code, message, fields } = await refusal(insertDuty({ [field]: 1.5 })); + expect(code).toBe('VALIDATION_FAILED'); + expect(fields).toHaveLength(1); + expect(fields[0]).toMatchObject({ field, code: 'max_scale', constraint: { scale: 0, actual: 1 } }); + // The refusal names the field the author is looking at, in its own label. + expect(message).toContain(String((Duty.fields[field] as { label?: string }).label)); + expect(message).toContain('at most 0 decimal places'); + }); + + it.each(CADENCE_FIELDS)('duly_catalog_item.%s: 1.5 is refused too', async (field) => { + const { code, fields } = await refusal(insertCatalogItem({ [field]: 1.5 })); + expect(code).toBe('VALIDATION_FAILED'); + expect(fields[0]).toMatchObject({ field, code: 'max_scale' }); + }); + + it('the UPDATE path refuses it as well — not just insert', async () => { + // #65's lesson: a rule that holds on insert and does nothing on update + // leaves exactly one reachable path to the broken state, and it is the + // one a configurer actually uses when fixing a cadence. + const created: any = await insertDuty({}); + const row = Array.isArray(created) ? created[0] : created; + const { code, fields } = await refusal( + data.update('duly_duty', { id: row.id, due_offset_days: 1.5 }), + ); + expect(code).toBe('VALIDATION_FAILED'); + expect(fields[0]).toMatchObject({ field: 'due_offset_days', code: 'max_scale' }); + }); +}); + +describe('an out-of-range cadence value is refused at write time', () => { + it.each(CADENCE_FIELDS)('duly_duty.%s refuses max + 1', async (field) => { + const { code, fields } = await refusal(insertDuty({ [field]: BOUNDS[field].max + 1 })); + expect(code).toBe('VALIDATION_FAILED'); + expect(fields[0]).toMatchObject({ field, code: 'max_value', constraint: { max: BOUNDS[field].max } }); + }); + + it.each(CADENCE_FIELDS)('duly_duty.%s refuses min - 1', async (field) => { + const { code, fields } = await refusal(insertDuty({ [field]: BOUNDS[field].min - 1 })); + expect(code).toBe('VALIDATION_FAILED'); + expect(fields[0]).toMatchObject({ field, code: 'min_value', constraint: { min: BOUNDS[field].min } }); + }); + + it('9e9 — the card\'s own example — is refused here rather than deep in the engine', async () => { + // Before this card it saved clean and failed at dispatch, as + // "dueDate must be a YYYY-MM-DD calendar date, received 0NaN-NaN-NaN": + // a message naming neither the field nor anything the author typed. (The + // card predicted the MIN_YEAR/MAX_YEAR guard would catch it; measured, it + // did not get that far — the civil arithmetic went to NaN first.) + const { fields } = await refusal(insertDuty({ due_offset_days: 9e9 })); + expect(fields[0]).toMatchObject({ field: 'due_offset_days', code: 'max_value' }); + }); +}); + +describe('every value still admitted is one the product can carry out', () => { + // A bound that refuses a legal value is as wrong as one that admits an + // illegal one, so the corners are asserted to LAND, not just the middle. + it.each([ + ['both extremes of the offset, with the maximum lead', { due_offset_days: -366, lead_days: 366 }], + ['the other extreme', { due_offset_days: 366, lead_days: 0 }], + ['the largest grace the sweep can honour', { grace_days: 14 }], + ['zero everywhere', { due_offset_days: 0, lead_days: 0, grace_days: 0 }], + ] as const)('accepts %s', async (_label, values) => { + const created: any = await insertDuty(values); + const row = Array.isArray(created) ? created[0] : created; + for (const [k, v] of Object.entries(values)) expect(row[k]).toBe(v); + }); + + it('the period engine accepts every corner of the declared box', () => { + // This is the half that makes the bounds a CONTRACT rather than a guess: + // the declared box has to sit inside what `dispatch.plan.ts` can actually + // compute, or the card's defect is merely moved rather than closed. Every + // frequency, both anchors, both zones, offsets and leads at their + // declared extremes — and not one `invalid_cadence` among them. + const base: DispatchDuty = { + id: 'd1', name: 'D', form: 'recurring', status: 'active', owner: 'u1', + business_unit: null, source: 'self', frequency: 'monthly', + due_anchor: 'period_start', due_offset_days: 0, lead_days: 7, + timezone: 'UTC', effective_from: null, effective_to: null, last_dispatched_period: null, + }; + const now = new Date('2026-08-15T10:00:00.000Z'); + const faults: string[] = []; + for (const frequency of FREQUENCIES) { + for (const due_anchor of ['period_start', 'period_end']) { + for (const timezone of ['UTC', 'Europe/Berlin']) { + for (const due_offset_days of [BOUNDS.due_offset_days.min, 0, BOUNDS.due_offset_days.max]) { + for (const lead_days of [BOUNDS.lead_days.min, 7, BOUNDS.lead_days.max]) { + const plan = planDispatch({ + duties: [{ ...base, frequency, due_anchor, timezone, due_offset_days, lead_days }], + now, + }); + for (const skip of plan.skipped) { + if (skip.reason === 'invalid_cadence') { + faults.push(`${frequency}/${due_anchor}/${timezone}/${due_offset_days}/${lead_days}: ${skip.detail}`); + } + } + } + } + } + } + } + expect(faults).toEqual([]); + }); +}); diff --git a/test/reminders.test.ts b/test/reminders.test.ts index 03a75a6..065a33b 100644 --- a/test/reminders.test.ts +++ b/test/reminders.test.ts @@ -9,7 +9,7 @@ import { dulyFlows, dulyReminderFlows, } from '../src/flows/index.js'; -import { Duty, Task } from '../src/objects/index.js'; +import { CatalogItem, Duty, Task } from '../src/objects/index.js'; import { planDispatch } from '../src/jobs/dispatch.plan.js'; /** @@ -202,18 +202,28 @@ describe('reminder sweeps — the windows say what the card says', () => { it('the lookback still covers the largest grace a duty can declare', () => { // The escalation day is `due_date + grace_days + 1`, so a duty whose grace // pushes that day outside the swept window is never escalated — silently. - // `grace_days` declares `min: 0` and no maximum, so nothing but this - // assertion holds the two numbers together. + // Nothing but this assertion holds the two numbers together: they live in + // two files (`src/flows/reminders.flow.ts`, `src/objects/duty.object.ts`) + // and neither can see the other. const lookback = -Number(timeRelativeOf(OverdueOwnerEscalation as unknown as FlowLike).withinDays); const graceMax = (Duty.fields.grace_days as { max?: number }).max; expect( - graceMax === undefined || graceMax + 1 <= lookback, + graceMax !== undefined && graceMax + 1 <= lookback, `duly_duty.grace_days declares max ${String(graceMax)}, which needs a lookback of at least ` + `${Number(graceMax) + 1} days; the sweep looks back ${lookback}`, ).toBe(true); - // And the unbounded case, stated so it is a known limit and not a surprise: - // grace ≥ lookback is out of range whatever the field says. - expect(graceMax, 'grace_days grew a max — re-read the coupling above').toBeUndefined(); + // The direction the previous version of this test guarded — an UNBOUNDED + // grace — is now closed by declaration (#82), and this half is what keeps + // it closed. Deleting the field's `max` would put the silent case back: + // every value above 14 saves clean and is never escalated, which is not a + // gap a reader of either file would notice. + expect(graceMax, 'grace_days lost its max — an unbounded grace is silently never escalated').toBe( + lookback - 1, + ); + // Both objects declare the same ceiling. `duly_catalog_item.grace_days` is + // copied onto every duty an apply creates, so a laxer bound there is the + // same silent hole reached one object earlier. + expect((CatalogItem.fields.grace_days as { max?: number }).max).toBe(graceMax); }); });