diff --git a/.changeset/approval-escalation-calendar-clock.md b/.changeset/approval-escalation-calendar-clock.md new file mode 100644 index 0000000000..6eeb9924da --- /dev/null +++ b/.changeset/approval-escalation-calendar-clock.md @@ -0,0 +1,16 @@ +--- +"@objectstack/spec": patch +--- + +docs(spec): `ApprovalEscalation.timeoutHours` names its clock — calendar (wall-clock) hours + +The `timeoutHours` describe text now states that the hours are calendar +(wall-clock) hours: nights, weekends and holidays count, because the platform +ships no business-hours calendar, so a request opened at 17:00 on a Friday with +`timeoutHours: 4` escalates at 21:00 that same Friday. The sentence is published +contract text — it is what `gen:schema` emits to the JSON schema `description` +and what the reference page carries — so the unit is part of the declaration an +author reads at authoring time rather than prose beside it. No key is added, +renamed or defaulted differently; the approvals service's arithmetic is +unchanged and is now pinned by a wall-clock test (Friday 17:00 + 4 h, a 168-hour +deadline across a weekend, a DST transition). diff --git a/content/docs/automation/approvals.mdx b/content/docs/automation/approvals.mdx index af603b221c..3e85ba17e5 100644 --- a/content/docs/automation/approvals.mdx +++ b/content/docs/automation/approvals.mdx @@ -630,6 +630,10 @@ that request's drawer directly instead of a generic list. and pick an `action` — `notify` (default), `reassign`, `auto_approve`, or `auto_reject`. Auto decisions run through the normal decide path, so the flow resumes exactly as if a human had clicked. Every escalation writes an audit row. +`timeoutHours` counts **calendar (wall-clock) hours** — nights, weekends and +holidays included, because the platform ships no business-hours calendar — so a +request opened at 17:00 on a Friday with `timeoutHours: 4` escalates at 21:00 +that same Friday. **Escalation needs the job service.** The plugin sweeps pending requests on an diff --git a/content/docs/permissions/system-context.mdx b/content/docs/permissions/system-context.mdx index 88108c416a..effe83fd6f 100644 --- a/content/docs/permissions/system-context.mdx +++ b/content/docs/permissions/system-context.mdx @@ -145,7 +145,7 @@ The largest single consumer — **20 of the 109 sites**. |:--|:---|:---|:---|:---| | 40 | **Approval record lock released** — a locked record is writable | plugin-approvals | Get: engine self-writes (the status mirror) pass. Lose: the lock that stops edits while an approval is live. Note there is deliberately **no admin exemption** here — only `isSystem` | `lifecycle-hooks.ts:347` | | 41 | Delegation write guard bypassed | plugin-approvals | Get: service / seed / import may write delegation rows naming another delegator | `lifecycle-hooks.ts:570` | -| 42 | Approval actor / submitter / pending-approver checks bypassed (8 sites) | plugin-approvals | Get: approve, reject, recall, reassign without being a pending approver or the submitter | `plugin-approvals/src/approval-service.ts:950`, `:1059`, `:3163`, `:3309`, `:3476`, `:3547`, `:3736`, `:3776` | +| 42 | Approval actor / submitter / pending-approver checks bypassed (8 sites) | plugin-approvals | Get: approve, reject, recall, reassign without being a pending approver or the submitter | `plugin-approvals/src/approval-service.ts:963`, `:1072`, `:3176`, `:3322`, `:3489`, `:3560`, `:3749`, `:3789` | | 43 | Saved-report ownership is **assignable**, and an update may reassign it | plugin-reports | Get: `ownerId` from input is honoured. A non-system caller always owns what it creates and can never reassign | `plugin-reports/src/report-service.ts:404`, `:425` | | 44 | Saved-report access / export / mutation gates bypassed | plugin-reports | Get: read, bulk-export and overwrite any report | `plugin-reports/src/report-service.ts:343`, `:372`, `:447`, `:684` | | 45 | Attachment access hooks return early (insert + update + delete, and the read AST) | service-storage | Lose: attachment visibility scoping | `attachment-access-hooks.ts:300`, `:349`, `:448`, `:524` | diff --git a/content/docs/references/automation/approval.mdx b/content/docs/references/automation/approval.mdx index 7946a3989e..b318759db5 100644 --- a/content/docs/references/automation/approval.mdx +++ b/content/docs/references/automation/approval.mdx @@ -38,7 +38,7 @@ const result = ApprovalDecision.parse(data); | Property | Type | Required | Description | | :--- | :--- | :--- | :--- | | **enabled** | `boolean` | optional (default: `true`) | SLA escalation switch. Defaults to true: an escalation block carrying timeoutHours is live unless this is explicitly false — the feature-level switch is whether the escalation block exists at all | -| **timeoutHours** | `number` | ✅ | Hours before escalation triggers | +| **timeoutHours** | `number` | ✅ | Calendar (wall-clock) hours before escalation triggers — nights, weekends and holidays count. The platform ships no business-hours calendar: a request opened at 17:00 on a Friday with timeoutHours 4 escalates at 21:00 that same Friday | | **action** | `Enum<'reassign' \| 'auto_approve' \| 'auto_reject' \| 'notify'>` | optional (default: `"notify"`) | Action on escalation timeout | | **escalateTo** | `string` | optional | User id or position machine name to escalate to | | **notifySubmitter** | `boolean` | optional (default: `true`) | Notify the original submitter on escalation | @@ -102,7 +102,7 @@ const result = ApprovalDecision.parse(data); | Property | Type | Required | Description | | :--- | :--- | :--- | :--- | | **enabled** | `boolean` | optional (default: `true`) | SLA escalation switch. Defaults to true: an escalation block carrying timeoutHours is live unless this is explicitly false — the feature-level switch is whether the escalation block exists at all | -| **timeoutHours** | `number` | ✅ | Hours before escalation triggers | +| **timeoutHours** | `number` | ✅ | Calendar (wall-clock) hours before escalation triggers — nights, weekends and holidays count. The platform ships no business-hours calendar: a request opened at 17:00 on a Friday with timeoutHours 4 escalates at 21:00 that same Friday | | **action** | `Enum<'reassign' \| 'auto_approve' \| 'auto_reject' \| 'notify'>` | optional (default: `"notify"`) | Action on escalation timeout | | **escalateTo** | `string` | optional | User id or position machine name to escalate to | | **notifySubmitter** | `boolean` | optional (default: `true`) | Notify the original submitter on escalation | diff --git a/packages/plugins/plugin-approvals/src/approval-service-sla-calendar-clock.test.ts b/packages/plugins/plugin-approvals/src/approval-service-sla-calendar-clock.test.ts new file mode 100644 index 0000000000..f6ac3ac593 --- /dev/null +++ b/packages/plugins/plugin-approvals/src/approval-service-sla-calendar-clock.test.ts @@ -0,0 +1,208 @@ +// Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license. + +/** + * `ApprovalEscalation.timeoutHours` is CALENDAR (wall-clock) hours — pinned + * through the real code path, not restated. + * + * The declaration's `describe` text on `ApprovalEscalationSchema` says the + * clock out loud; `slaDueAt` in `approval-service.ts` is the one runtime site + * that turns the declared number into a deadline; the escalation sweep compares + * that deadline against the injected clock. This file drives all three through + * `openNodeRequest` → `getRequest` → `runEscalations`, so the sentence in the + * schema and the arithmetic in the service cannot drift apart without a red + * here. + * + * Timezone assumption, stated: NONE is required. Every timestamp the service + * reads or writes is an ISO-8601 UTC string (`toISOString()` / `Date.parse` of + * a `Z`-suffixed literal) and the deadline is `created_at` plus elapsed + * milliseconds, so the assertions hold under any `TZ` the runner sets — they + * are written against UTC instants and never call a local-time accessor. The + * DST cases document what the SAME instants read as on a wall clock in + * America/New_York, to make the elapsed-time-versus-local-time distinction + * visible where a reader would otherwise infer it. + */ + +import { describe, it, expect } from 'vitest'; +import { ApprovalService } from './approval-service.js'; + +interface Row { [k: string]: any } + +/** + * Read-and-append engine double: `find` + `insert` only. + * + * The three paths under test dispatch nothing else — `openNodeRequest` finds + * and inserts, `getRequest` finds, and the `notify` escalation arm finds and + * inserts the audit action. No `update` / `delete` member exists on purpose: + * `check:engine-double-contract` pins those write verbs to the real engine's + * dispatch, and a double that does not declare them has nothing to pin. + */ +function makeEngine() { + const tables: Record = {}; + const ensure = (n: string) => (tables[n] ??= []); + const matches = (row: Row, filter: any): boolean => { + if (!filter || typeof filter !== 'object') return true; + for (const [k, v] of Object.entries(filter)) { + if (k === '$or') { + if (!(v as any[]).some((sub) => matches(row, sub))) return false; + continue; + } + if (k.startsWith('$')) throw new Error(`fake engine: unsupported filter operator ${k}`); + const rv = row[k]; + if (v != null && typeof v === 'object' && '$in' in (v as any)) { + if (!(v as any).$in.includes(rv)) return false; + continue; + } + if (v != null && typeof v === 'object' && '$ne' in (v as any)) { + if (rv === (v as any).$ne) return false; + continue; + } + if (rv !== v) return false; + } + return true; + }; + return { + _tables: tables, + async find(object: string, options?: any) { + const rows = ensure(object).filter((r) => matches(r, options?.filter ?? options?.where)); + // Honour the caller's bound by presence, after the filter — the shape + // `check:objectql-double-limit` pins, so a `limit: 1` read cannot be + // silently answered with every row. + return typeof options?.limit === 'number' ? rows.slice(0, options.limit) : rows; + }, + async insert(object: string, data: Row) { ensure(object).push({ ...data }); return { ...data }; }, + async count(object: string) { return ensure(object).length; }, + registerHook() { /* no-op */ }, + unregisterHooksByPackage() { /* no-op */ }, + }; +} + +const HOUR = 3_600_000; +const SYS = { isSystem: true, positions: [], permissions: [] } as any; +const CTX = { userId: 'u1', tenantId: 't1', positions: [], permissions: [] } as any; + +/** A node whose only escalation dependency is the clock: `notify`, no reassign. */ +function input(nodeId: string, timeoutHours: number) { + return { + object: 'opportunity', + recordId: 'opp1', + runId: 'run_1', + nodeId, + flowName: 'deal_approval', + config: { + approvers: [{ type: 'user' as const, value: 'u9' }], + behavior: 'first_response' as const, + lockRecord: false, + escalation: { timeoutHours, action: 'notify' as const, escalateTo: 'boss', notifySubmitter: false }, + }, + record: { id: 'opp1', amount: 100 }, + }; +} + +/** + * Open a node request and return the PENDING row. `openNodeRequest` can also + * answer with an auto outcome (an empty approver slate under + * `onEmptyApprovers: 'auto_approve'`), which carries no `id` and no SLA — the + * arm this file is not about, so it is refused loudly rather than narrowed + * away with a cast. + */ +async function openPending(svc: ApprovalService, nodeInput: ReturnType) { + const opened = await svc.openNodeRequest(nodeInput, CTX); + if (!('id' in opened)) throw new Error('expected a pending approval request, got an auto outcome'); + return opened; +} + +/** A service whose clock is set by the test, in UTC instants. */ +function serviceAt(iso: string) { + let nowMs = Date.parse(iso); + const engine = makeEngine(); + const svc = new ApprovalService({ engine: engine as any, clock: { now: () => new Date(nowMs) } }); + return { svc, engine, setNow: (at: string) => { nowMs = Date.parse(at); } }; +} + +const utcDay = (iso: string) => new Date(iso).getUTCDay(); // 0 = Sunday … 5 = Friday, 6 = Saturday + +// 2026-01-16 is a Friday; the calendar claims below are about the dates they name. +const FRIDAY_1700 = '2026-01-16T17:00:00.000Z'; +const MONDAY_0900 = '2026-01-19T09:00:00.000Z'; + +describe('ApprovalEscalation.timeoutHours is calendar (wall-clock) hours', () => { + it('the fixture dates are the weekdays the assertions name', () => { + expect(utcDay(FRIDAY_1700)).toBe(5); + expect(utcDay(MONDAY_0900)).toBe(1); + }); + + it('Friday 17:00 + timeoutHours 4 is due Friday 21:00 — the same evening, not the next business day', async () => { + const { svc, setNow } = serviceAt(FRIDAY_1700); + const req = await openPending(svc, input('sla_4h', 4)); + + const row = await svc.getRequest(req.id, SYS); + expect(row?.created_at).toBe(FRIDAY_1700); + expect(row?.sla_due_at).toBe('2026-01-16T21:00:00.000Z'); + expect(utcDay(row!.sla_due_at!)).toBe(5); + // A business-hours reading would put this deadline on Monday at the + // earliest; the wall clock puts it before Monday's first working hour. + expect(Date.parse(row!.sla_due_at!)).toBeLessThan(Date.parse(MONDAY_0900)); + + // The sweep reads the same deadline: one millisecond early is not overdue, + // the deadline instant itself is — on Friday night, with nobody at work. + setNow('2026-01-16T20:59:59.999Z'); + expect(await svc.runEscalations()).toMatchObject({ escalated: 0 }); + setNow('2026-01-16T21:00:00.000Z'); + expect(await svc.runEscalations()).toMatchObject({ escalated: 1 }); + + const actions = await svc.listActions(req.id, SYS); + expect(actions.at(-1)).toMatchObject({ action: 'escalate', actor_id: 'system:sla' }); + }); + + it('a 168-hour deadline spans the weekend: due the next Friday at the same hour, 7 × 24 elapsed hours', async () => { + const { svc, setNow } = serviceAt(FRIDAY_1700); + const req = await openPending(svc, input('sla_168h', 168)); + + const row = await svc.getRequest(req.id, SYS); + const due = row!.sla_due_at!; + expect(due).toBe('2026-01-23T17:00:00.000Z'); + expect(utcDay(due)).toBe(5); + expect(Date.parse(due) - Date.parse(FRIDAY_1700)).toBe(168 * HOUR); + + // Saturday and Sunday sit inside the window and are not skipped: the + // deadline is not 168 working hours later (that would be four weeks out). + const saturday = '2026-01-17T12:00:00.000Z'; + const sunday = '2026-01-18T12:00:00.000Z'; + expect(utcDay(saturday)).toBe(6); + expect(utcDay(sunday)).toBe(0); + for (const weekendInstant of [saturday, sunday]) { + expect(Date.parse(weekendInstant)).toBeGreaterThan(Date.parse(FRIDAY_1700)); + expect(Date.parse(weekendInstant)).toBeLessThan(Date.parse(due)); + } + + setNow(MONDAY_0900); + expect(await svc.runEscalations()).toMatchObject({ escalated: 0 }); + setNow(due); + expect(await svc.runEscalations()).toMatchObject({ escalated: 1 }); + }); + + it('a DST transition changes nothing: elapsed hours, not local wall-clock hours (spring forward)', async () => { + // 2026-03-08T05:00:00Z is 00:00 EST in America/New_York; at 02:00 local the + // clocks jump to 03:00 EDT. Four ELAPSED hours later is 09:00Z = 05:00 EDT — + // five o'clock on the local wall, four hours of real time. The service adds + // elapsed milliseconds, so the deadline is the 09:00Z instant on every host. + const created = '2026-03-08T05:00:00.000Z'; + const { svc } = serviceAt(created); + const req = await openPending(svc, input('sla_dst_spring', 4)); + const row = await svc.getRequest(req.id, SYS); + expect(row?.sla_due_at).toBe('2026-03-08T09:00:00.000Z'); + expect(Date.parse(row!.sla_due_at!) - Date.parse(created)).toBe(4 * HOUR); + }); + + it('a DST transition changes nothing: elapsed hours, not local wall-clock hours (fall back)', async () => { + // 2026-11-01T05:00:00Z is 01:00 EDT in America/New_York; at 02:00 EDT the + // clocks go back to 01:00 EST. Four ELAPSED hours later is 09:00Z = 04:00 + // EST — three o'clock-hours on the local wall, four hours of real time. + const created = '2026-11-01T05:00:00.000Z'; + const { svc } = serviceAt(created); + const req = await openPending(svc, input('sla_dst_fall', 4)); + const row = await svc.getRequest(req.id, SYS); + expect(row?.sla_due_at).toBe('2026-11-01T09:00:00.000Z'); + expect(Date.parse(row!.sla_due_at!) - Date.parse(created)).toBe(4 * HOUR); + }); +}); diff --git a/packages/plugins/plugin-approvals/src/approval-service.ts b/packages/plugins/plugin-approvals/src/approval-service.ts index fd8a406788..7d44eb9e69 100644 --- a/packages/plugins/plugin-approvals/src/approval-service.ts +++ b/packages/plugins/plugin-approvals/src/approval-service.ts @@ -607,7 +607,20 @@ function rowFromRequest(row: any): ApprovalRequestRow { } as any; } -/** `created_at + escalation.timeoutHours`, when the node declares an SLA. */ +/** + * `created_at + escalation.timeoutHours`, when the node declares an SLA. + * + * Calendar (wall-clock) hours, by construction: the hours are added as elapsed + * milliseconds, so the deadline does not skip nights, weekends or holidays — + * the platform ships no business-hours calendar to count against. This is the + * one runtime site that turns the declared number into a deadline; the sweep + * below and the `sla_due_at` read projection both go through it, and + * `approval-service-sla-calendar-clock.test.ts` pins the clock (a request + * opened Friday 17:00 with `timeoutHours: 4` is due Friday 21:00; a 168-hour + * deadline spans the weekend; a DST transition changes nothing, because the + * arithmetic is elapsed time, not local calendar time). The same sentence + * lives in the declaration's `describe` text on `ApprovalEscalationSchema`. + */ function slaDueAt(createdAt: unknown, cfg: any): string | undefined { const hours = cfg?.escalation?.timeoutHours; if (typeof hours !== 'number' || hours <= 0 || !createdAt) return undefined; diff --git a/packages/spec/src/automation/approval.zod.ts b/packages/spec/src/automation/approval.zod.ts index 2818df6db1..b829fcec4f 100644 --- a/packages/spec/src/automation/approval.zod.ts +++ b/packages/spec/src/automation/approval.zod.ts @@ -620,7 +620,17 @@ export const ApprovalEscalationSchema = lazySchema(() => strictObject( // surface have always meant. Declared in DEFAULT_CHANGES_BY_MAJOR (17) and // the `approval-escalation-enabled-default-flip` semantic migration entry. enabled: z.boolean().default(true).describe('SLA escalation switch. Defaults to true: an escalation block carrying timeoutHours is live unless this is explicitly false — the feature-level switch is whether the escalation block exists at all'), - timeoutHours: z.number().min(1).describe('Hours before escalation triggers'), + /** + * Wall-clock SLA. The approvals service adds `timeoutHours` to the request's + * `created_at` as elapsed milliseconds (hours × 3_600_000), so the deadline + * does not skip nights, weekends or holidays — the platform ships no + * business-hours calendar to count against. The clock is named in the + * declaration's own contract text rather than in prose beside it, so the + * number cannot be read as working hours at authoring time. No `clock` key + * exists because only one clock exists: a key with a single legal value would + * be declared-but-inert (ADR-0049). + */ + timeoutHours: z.number().min(1).describe('Calendar (wall-clock) hours before escalation triggers — nights, weekends and holidays count. The platform ships no business-hours calendar: a request opened at 17:00 on a Friday with timeoutHours 4 escalates at 21:00 that same Friday'), action: z.enum(['reassign', 'auto_approve', 'auto_reject', 'notify']).default('notify') .describe('Action on escalation timeout'), // Escalation hands the request to a position (the common case — e.g. an