diff --git a/.changeset/escalate-to-position.md b/.changeset/escalate-to-position.md new file mode 100644 index 0000000000..8db880f355 --- /dev/null +++ b/.changeset/escalate-to-position.md @@ -0,0 +1,19 @@ +--- +'@objectstack/spec': patch +'@objectstack/plugin-approvals': minor +'@objectstack/lint': minor +--- + +SLA escalation `escalateTo` is position-first (ADR-0090 D3 follow-up to the `position` approver type). + +- **spec**: `ApprovalEscalationSchema.escalateTo` is documented as a position machine name or a + specific user id (was "User id, role, or manager level" — the same pre-D3 'role' trap the + `position` approver type fixed); the Studio xRef picker kind moves `role` → `position`. +- **plugin-approvals**: on escalation, `escalateTo` now expands position holders via + `sys_user_position` ∪ the `sys_member.role` transition source (ADR-0057 D4) for both the + `reassign` approver hand-off and the `notify` audience. An empty expansion falls back to + treating the value as a literal user id, so configs naming a specific user keep working + unchanged. The audit trail keeps the authored target. +- **lint**: new `approval-escalation-reassign-no-target` warning — `escalation.action: 'reassign'` + with no `escalateTo` silently degrades to a notify at runtime; the fix-it prescribes a position + or user id target (or `action: 'notify'`). diff --git a/content/docs/references/automation/approval.mdx b/content/docs/references/automation/approval.mdx index 2c94a136d8..540685e9d6 100644 --- a/content/docs/references/automation/approval.mdx +++ b/content/docs/references/automation/approval.mdx @@ -42,7 +42,7 @@ const result = ApprovalDecision.parse(data); | **enabled** | `boolean` | ✅ | Enable SLA-based escalation for this node | | **timeoutHours** | `number` | ✅ | Hours before escalation triggers | | **action** | `Enum<'reassign' \| 'auto_approve' \| 'auto_reject' \| 'notify'>` | ✅ | Action on escalation timeout | -| **escalateTo** | `string` | optional | User id, role, or manager level to escalate to | +| **escalateTo** | `string` | optional | User id or position machine name to escalate to | | **notifySubmitter** | `boolean` | ✅ | Notify the original submitter on escalation | diff --git a/packages/lint/src/index.ts b/packages/lint/src/index.ts index 40763890c0..f94d16a391 100644 --- a/packages/lint/src/index.ts +++ b/packages/lint/src/index.ts @@ -79,6 +79,7 @@ export { validateApprovalApprovers, APPROVAL_ROLE_NOT_MEMBERSHIP_TIER, APPROVAL_APPROVER_TYPE_UNKNOWN, + APPROVAL_ESCALATION_REASSIGN_NO_TARGET, } from './validate-approval-approvers.js'; export type { ApprovalApproverFinding, ApprovalApproverSeverity } from './validate-approval-approvers.js'; diff --git a/packages/lint/src/validate-approval-approvers.test.ts b/packages/lint/src/validate-approval-approvers.test.ts index ba45a588d8..0de761901f 100644 --- a/packages/lint/src/validate-approval-approvers.test.ts +++ b/packages/lint/src/validate-approval-approvers.test.ts @@ -5,6 +5,7 @@ import { validateApprovalApprovers, APPROVAL_ROLE_NOT_MEMBERSHIP_TIER, APPROVAL_APPROVER_TYPE_UNKNOWN, + APPROVAL_ESCALATION_REASSIGN_NO_TARGET, } from './validate-approval-approvers.js'; function stackWithApprovers(approvers: unknown[]): Record { @@ -72,6 +73,26 @@ describe('validateApprovalApprovers', () => { expect(findings[1].rule).toBe(APPROVAL_APPROVER_TYPE_UNKNOWN); }); + it("flags escalation.action 'reassign' with no escalateTo (silent notify degradation)", () => { + const stack = stackWithApprovers([{ type: 'user', value: 'u1' }]); + const node = (stack.flows as any)[0].nodes[1]; + node.config.escalation = { enabled: true, timeoutHours: 24, action: 'reassign' }; + const findings = validateApprovalApprovers(stack); + expect(findings).toHaveLength(1); + expect(findings[0].rule).toBe(APPROVAL_ESCALATION_REASSIGN_NO_TARGET); + expect(findings[0].path).toBe('flows[0].nodes[1].config.escalation.escalateTo'); + expect(findings[0].hint).toContain('position'); + }); + + it('accepts reassign escalation with a target, and non-reassign actions without one', () => { + const stack = stackWithApprovers([{ type: 'user', value: 'u1' }]); + const node = (stack.flows as any)[0].nodes[1]; + node.config.escalation = { enabled: true, timeoutHours: 24, action: 'reassign', escalateTo: 'approvals_supervisor' }; + expect(validateApprovalApprovers(stack)).toEqual([]); + node.config.escalation = { enabled: true, timeoutHours: 24, action: 'notify' }; + expect(validateApprovalApprovers(stack)).toEqual([]); + }); + it('only scans approval nodes and tolerates malformed shapes', () => { const findings = validateApprovalApprovers({ flows: [{ diff --git a/packages/lint/src/validate-approval-approvers.ts b/packages/lint/src/validate-approval-approvers.ts index 76caf7deb6..32a1bc2523 100644 --- a/packages/lint/src/validate-approval-approvers.ts +++ b/packages/lint/src/validate-approval-approvers.ts @@ -14,10 +14,11 @@ * * Rules: * - * | Rule | Severity | Origin | - * |-------------------------------------|----------|----------------------------| - * | approval-role-not-membership-tier | warning | ADR-0090 D3 (hotcrm class) | - * | approval-approver-type-unknown | warning | contract-first (PD #12) | + * | Rule | Severity | Origin | + * |--------------------------------------------|----------|----------------------------| + * | approval-role-not-membership-tier | warning | ADR-0090 D3 (hotcrm class) | + * | approval-approver-type-unknown | warning | contract-first (PD #12) | + * | approval-escalation-reassign-no-target | warning | silent notify degradation | * * Warnings (not errors): a custom better-auth membership tier is legal, and * the runtime keeps its literal fallback — but both shapes are near-certainly @@ -30,6 +31,7 @@ import { ApproverType, APPROVAL_NODE_TYPE } from '@objectstack/spec/automation'; export const APPROVAL_ROLE_NOT_MEMBERSHIP_TIER = 'approval-role-not-membership-tier'; export const APPROVAL_APPROVER_TYPE_UNKNOWN = 'approval-approver-type-unknown'; +export const APPROVAL_ESCALATION_REASSIGN_NO_TARGET = 'approval-escalation-reassign-no-target'; export type ApprovalApproverSeverity = 'error' | 'warning' | 'info'; @@ -136,6 +138,28 @@ export function validateApprovalApprovers(stack: AnyRec): ApprovalApproverFindin }); } } + + // escalation.action 'reassign' with no escalateTo silently degrades to a + // plain SLA-breach notification at runtime — the hand-off the author + // asked for never happens. + const escalation = (cfg.escalation ?? null) as AnyRec | null; + if (escalation && typeof escalation === 'object' && escalation.action === 'reassign') { + const target = typeof escalation.escalateTo === 'string' ? escalation.escalateTo.trim() : ''; + if (!target) { + findings.push({ + severity: 'warning', + rule: APPROVAL_ESCALATION_REASSIGN_NO_TARGET, + where, + path: `flows[${fi}].nodes[${ni}].config.escalation.escalateTo`, + message: + `escalation.action is 'reassign' but escalateTo is empty — at runtime the ` + + `escalation degrades to a notify and the request stays with the original approvers.`, + hint: + `Set escalateTo to a position machine name (expanded via sys_user_position, ` + + `ADR-0090 D3) or a specific user id, or change action to 'notify'.`, + }); + } + } } } diff --git a/packages/plugins/plugin-approvals/src/approval-service.test.ts b/packages/plugins/plugin-approvals/src/approval-service.test.ts index d65a93c18c..55078feb1a 100644 --- a/packages/plugins/plugin-approvals/src/approval-service.test.ts +++ b/packages/plugins/plugin-approvals/src/approval-service.test.ts @@ -863,6 +863,40 @@ describe('ApprovalService (node era)', () => { expect(fresh?.pending_approvers).toEqual(['boss']); }); + it('runEscalations: reassign expands a position escalateTo to its holders (ADR-0090 D3)', async () => { + engine._tables['sys_user_position'] = [ + { id: 'up1', user_id: 'u5', position: 'approvals_supervisor', organization_id: 't1' }, + { id: 'up2', user_id: 'u6', position: 'approvals_supervisor', organization_id: 't1' }, + { id: 'up3', user_id: 'u7', position: 'approvals_supervisor', organization_id: 't2' }, // other tenant + ]; + const req = await svc.openNodeRequest( + openInput(['u9'], {}, { escalation: { timeoutHours: 1, action: 'reassign', escalateTo: 'approvals_supervisor', notifySubmitter: false } }), CTX, + ); + makeOverdue(req.id); + await svc.runEscalations(); + const fresh = await svc.getRequest(req.id, SYS); + expect(fresh?.status).toBe('pending'); + expect(fresh?.pending_approvers?.slice().sort()).toEqual(['u5', 'u6']); + // The audit trail keeps the AUTHORED target, not the expansion. + const actions = await svc.listActions(req.id, SYS); + expect(actions.find(a => a.action === 'escalate')?.comment).toBe('reassign → approvals_supervisor'); + }); + + it('runEscalations: notify expands a position escalateTo into the audience', async () => { + engine._tables['sys_user_position'] = [ + { id: 'up1', user_id: 'u5', position: 'approvals_supervisor', organization_id: 't1' }, + ]; + const emitted: any[] = []; + svc.attachMessaging({ async emit(input) { emitted.push(input); } }); + const req = await svc.openNodeRequest( + openInput(['u9'], {}, { escalation: { timeoutHours: 2, action: 'notify', escalateTo: 'approvals_supervisor', notifySubmitter: false } }), CTX, + ); + makeOverdue(req.id); + await svc.runEscalations(); + expect(emitted).toHaveLength(1); + expect(emitted[0].audience).toEqual(['u9', 'u5']); + }); + it('runEscalations: skips requests that are not yet due or have no SLA', async () => { await svc.openNodeRequest( openInput(['u9'], {}, { escalation: { timeoutHours: 1000, action: 'auto_approve' } }), CTX, diff --git a/packages/plugins/plugin-approvals/src/approval-service.ts b/packages/plugins/plugin-approvals/src/approval-service.ts index 39ae136323..5a389368de 100644 --- a/packages/plugins/plugin-approvals/src/approval-service.ts +++ b/packages/plugins/plugin-approvals/src/approval-service.ts @@ -1381,6 +1381,18 @@ export class ApprovalService implements IApprovalService { const now = this.clock.now().toISOString(); const pending = csvSplit(raw.pending_approvers); + // `escalateTo` is a position machine name or a user id (same contract as + // the `position` ApproverType, ADR-0090 D3). Position holders win; an + // empty expansion falls back to the literal, so a config naming a + // specific user id keeps working unchanged. + let escalatees: string[] = []; + if (escalateTo) { + try { + escalatees = await this.expandPositionUsers(escalateTo, raw.organization_id ?? null); + } catch { escalatees = []; } + if (!escalatees.length) escalatees = [escalateTo]; + } + // Audit first — this row IS the idempotency marker (ADR-0042 §1). await this.engine.insert('sys_approval_action', { id: uid('aact'), request_id: raw.id, organization_id: raw.organization_id ?? null, @@ -1390,14 +1402,14 @@ export class ApprovalService implements IApprovalService { created_at: now, }, { context: SYSTEM_CTX }); - if (action === 'reassign' && escalateTo) { + if (action === 'reassign' && escalatees.length) { await this.engine.update('sys_approval_request', { - id: raw.id, pending_approvers: escalateTo, updated_at: now, + id: raw.id, pending_approvers: escalatees.join(','), updated_at: now, }, { context: SYSTEM_CTX }); - await this.syncApproverIndex(raw.id, [escalateTo], raw.organization_id ?? null, now); + await this.syncApproverIndex(raw.id, escalatees, raw.organization_id ?? null, now); await this.notify({ topic: 'approval.escalated', - audience: [escalateTo], + audience: escalatees, actorId: SLA_ACTOR_ID, source: { object: 'sys_approval_request', id: raw.id }, payload: { @@ -1416,7 +1428,7 @@ export class ApprovalService implements IApprovalService { // 'notify' (and the reassign-without-target fallback) await this.notify({ topic: 'approval.sla_breached', - audience: [...pending, ...(escalateTo ? [escalateTo] : [])], + audience: [...pending, ...escalatees], actorId: SLA_ACTOR_ID, source: { object: 'sys_approval_request', id: raw.id }, payload: { diff --git a/packages/spec/src/automation/approval.zod.ts b/packages/spec/src/automation/approval.zod.ts index 44e30f25cf..22835049c2 100644 --- a/packages/spec/src/automation/approval.zod.ts +++ b/packages/spec/src/automation/approval.zod.ts @@ -119,12 +119,15 @@ export const ApprovalEscalationSchema = lazySchema(() => z.object({ timeoutHours: z.number().min(1).describe('Hours before escalation triggers'), action: z.enum(['reassign', 'auto_approve', 'auto_reject', 'notify']).default('notify') .describe('Action on escalation timeout'), - // Escalation hands the request to a role (the common case — e.g. a manager - // role or an approvals queue owner); the Studio designer renders a role - // picker, but free text is still accepted for a specific user id. + // Escalation hands the request to a position (the common case — e.g. an + // approvals supervisor); the Studio designer renders a position picker, but + // free text is still accepted for a specific user id. The engine expands a + // position machine name to its holders via `sys_user_position` (ADR-0090 + // D3) and falls back to treating the value as a user id when nobody holds + // it. NOT a better-auth membership tier — same contract as ApproverType. escalateTo: z.string().optional().meta({ - description: 'User id, role, or manager level to escalate to', - xRef: { kind: 'role' }, + description: 'User id or position machine name to escalate to', + xRef: { kind: 'position' }, }), notifySubmitter: z.boolean().default(true).describe('Notify the original submitter on escalation'), })); diff --git a/skills/objectstack-automation/SKILL.md b/skills/objectstack-automation/SKILL.md index 82a52d330c..dc5ad4eff5 100644 --- a/skills/objectstack-automation/SKILL.md +++ b/skills/objectstack-automation/SKILL.md @@ -392,7 +392,7 @@ branch — you never resume the flow by hand. | `behavior` | `first_response` (first approver decides) or `unanimous` (all must approve). Default `first_response` | | `lockRecord` | Lock the triggering record from edits while pending. Default `true` | | `approvalStatusField` | Business-object field to mirror `pending`/`approved`/`rejected`/`recalled` onto (should be readonly) | -| `escalation` | Optional per-node SLA — `{ enabled, timeoutHours, action: reassign\|auto_approve\|auto_reject\|notify, escalateTo?, notifySubmitter }` | +| `escalation` | Optional per-node SLA — `{ enabled, timeoutHours, action: reassign\|auto_approve\|auto_reject\|notify, escalateTo?, notifySubmitter }`. `escalateTo` is a **position machine name** (expanded to its holders via `sys_user_position`, ADR-0090 D3) or a specific user id — never a membership tier. `reassign` without `escalateTo` degrades to notify (linted) | | `maxRevisions` | ADR-0044 — max **send-backs-for-revision** per run before auto-reject. Default `3`; `0` disables send-back. Only meaningful when the node has a `revise` out-edge | ### Branching, side-effects & rejection