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

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
19 changes: 19 additions & 0 deletions .changeset/escalate-to-position.md
Original file line number Diff line number Diff line change
@@ -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'`).
2 changes: 1 addition & 1 deletion content/docs/references/automation/approval.mdx
Original file line number Diff line number Diff line change
Expand Up @@ -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 |


Expand Down
1 change: 1 addition & 0 deletions packages/lint/src/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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';

Expand Down
21 changes: 21 additions & 0 deletions packages/lint/src/validate-approval-approvers.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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<string, unknown> {
Expand Down Expand Up @@ -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: [{
Expand Down
32 changes: 28 additions & 4 deletions packages/lint/src/validate-approval-approvers.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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';

Expand Down Expand Up @@ -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'.`,
});
}
}
}
}

Expand Down
34 changes: 34 additions & 0 deletions packages/plugins/plugin-approvals/src/approval-service.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down
22 changes: 17 additions & 5 deletions packages/plugins/plugin-approvals/src/approval-service.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand All @@ -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: {
Expand All @@ -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: {
Expand Down
13 changes: 8 additions & 5 deletions packages/spec/src/automation/approval.zod.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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'),
}));
Expand Down
2 changes: 1 addition & 1 deletion skills/objectstack-automation/SKILL.md
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down