Skip to content

Commit 5a8465f

Browse files
os-zhuangclaude
andauthored
feat(approvals): SLA escalateTo resolves positions via sys_user_position (ADR-0090 D3 follow-up) (#2767)
escalateTo carried the same pre-D3 trap the 'position' approver type fixed: its description said 'User id, role, or manager level', its Studio picker was a role picker, and the runtime used the raw string verbatim — a position name authored there reassigned the request to an inert literal. - spec: escalateTo is a position machine name or user id; xRef kind role→position. - plugin-approvals: escalateRequest expands position holders (sys_user_position ∪ sys_member.role transition source) for the reassign hand-off and the notify audience; empty expansion falls back to the literal user id, so existing configs naming a user keep working. Audit keeps the authored target. - lint: approval-escalation-reassign-no-target warns when action 'reassign' has no escalateTo (silently degrades to notify at runtime). - docs/skill: SKILL.md escalation row spells out the contract; approval reference regenerated. Claude-Session: https://claude.ai/code/session_01DgP1vEK6nrkkvnwQxCPGbe Co-authored-by: Claude <noreply@anthropic.com>
1 parent 82ba3a6 commit 5a8465f

9 files changed

Lines changed: 130 additions & 16 deletions

File tree

.changeset/escalate-to-position.md

Lines changed: 19 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,19 @@
1+
---
2+
'@objectstack/spec': patch
3+
'@objectstack/plugin-approvals': minor
4+
'@objectstack/lint': minor
5+
---
6+
7+
SLA escalation `escalateTo` is position-first (ADR-0090 D3 follow-up to the `position` approver type).
8+
9+
- **spec**: `ApprovalEscalationSchema.escalateTo` is documented as a position machine name or a
10+
specific user id (was "User id, role, or manager level" — the same pre-D3 'role' trap the
11+
`position` approver type fixed); the Studio xRef picker kind moves `role``position`.
12+
- **plugin-approvals**: on escalation, `escalateTo` now expands position holders via
13+
`sys_user_position` ∪ the `sys_member.role` transition source (ADR-0057 D4) for both the
14+
`reassign` approver hand-off and the `notify` audience. An empty expansion falls back to
15+
treating the value as a literal user id, so configs naming a specific user keep working
16+
unchanged. The audit trail keeps the authored target.
17+
- **lint**: new `approval-escalation-reassign-no-target` warning — `escalation.action: 'reassign'`
18+
with no `escalateTo` silently degrades to a notify at runtime; the fix-it prescribes a position
19+
or user id target (or `action: 'notify'`).

content/docs/references/automation/approval.mdx

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -42,7 +42,7 @@ const result = ApprovalDecision.parse(data);
4242
| **enabled** | `boolean` || Enable SLA-based escalation for this node |
4343
| **timeoutHours** | `number` || Hours before escalation triggers |
4444
| **action** | `Enum<'reassign' \| 'auto_approve' \| 'auto_reject' \| 'notify'>` || Action on escalation timeout |
45-
| **escalateTo** | `string` | optional | User id, role, or manager level to escalate to |
45+
| **escalateTo** | `string` | optional | User id or position machine name to escalate to |
4646
| **notifySubmitter** | `boolean` || Notify the original submitter on escalation |
4747

4848

packages/lint/src/index.ts

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -79,6 +79,7 @@ export {
7979
validateApprovalApprovers,
8080
APPROVAL_ROLE_NOT_MEMBERSHIP_TIER,
8181
APPROVAL_APPROVER_TYPE_UNKNOWN,
82+
APPROVAL_ESCALATION_REASSIGN_NO_TARGET,
8283
} from './validate-approval-approvers.js';
8384
export type { ApprovalApproverFinding, ApprovalApproverSeverity } from './validate-approval-approvers.js';
8485

packages/lint/src/validate-approval-approvers.test.ts

Lines changed: 21 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,7 @@ import {
55
validateApprovalApprovers,
66
APPROVAL_ROLE_NOT_MEMBERSHIP_TIER,
77
APPROVAL_APPROVER_TYPE_UNKNOWN,
8+
APPROVAL_ESCALATION_REASSIGN_NO_TARGET,
89
} from './validate-approval-approvers.js';
910

1011
function stackWithApprovers(approvers: unknown[]): Record<string, unknown> {
@@ -72,6 +73,26 @@ describe('validateApprovalApprovers', () => {
7273
expect(findings[1].rule).toBe(APPROVAL_APPROVER_TYPE_UNKNOWN);
7374
});
7475

76+
it("flags escalation.action 'reassign' with no escalateTo (silent notify degradation)", () => {
77+
const stack = stackWithApprovers([{ type: 'user', value: 'u1' }]);
78+
const node = (stack.flows as any)[0].nodes[1];
79+
node.config.escalation = { enabled: true, timeoutHours: 24, action: 'reassign' };
80+
const findings = validateApprovalApprovers(stack);
81+
expect(findings).toHaveLength(1);
82+
expect(findings[0].rule).toBe(APPROVAL_ESCALATION_REASSIGN_NO_TARGET);
83+
expect(findings[0].path).toBe('flows[0].nodes[1].config.escalation.escalateTo');
84+
expect(findings[0].hint).toContain('position');
85+
});
86+
87+
it('accepts reassign escalation with a target, and non-reassign actions without one', () => {
88+
const stack = stackWithApprovers([{ type: 'user', value: 'u1' }]);
89+
const node = (stack.flows as any)[0].nodes[1];
90+
node.config.escalation = { enabled: true, timeoutHours: 24, action: 'reassign', escalateTo: 'approvals_supervisor' };
91+
expect(validateApprovalApprovers(stack)).toEqual([]);
92+
node.config.escalation = { enabled: true, timeoutHours: 24, action: 'notify' };
93+
expect(validateApprovalApprovers(stack)).toEqual([]);
94+
});
95+
7596
it('only scans approval nodes and tolerates malformed shapes', () => {
7697
const findings = validateApprovalApprovers({
7798
flows: [{

packages/lint/src/validate-approval-approvers.ts

Lines changed: 28 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -14,10 +14,11 @@
1414
*
1515
* Rules:
1616
*
17-
* | Rule | Severity | Origin |
18-
* |-------------------------------------|----------|----------------------------|
19-
* | approval-role-not-membership-tier | warning | ADR-0090 D3 (hotcrm class) |
20-
* | approval-approver-type-unknown | warning | contract-first (PD #12) |
17+
* | Rule | Severity | Origin |
18+
* |--------------------------------------------|----------|----------------------------|
19+
* | approval-role-not-membership-tier | warning | ADR-0090 D3 (hotcrm class) |
20+
* | approval-approver-type-unknown | warning | contract-first (PD #12) |
21+
* | approval-escalation-reassign-no-target | warning | silent notify degradation |
2122
*
2223
* Warnings (not errors): a custom better-auth membership tier is legal, and
2324
* 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';
3031

3132
export const APPROVAL_ROLE_NOT_MEMBERSHIP_TIER = 'approval-role-not-membership-tier';
3233
export const APPROVAL_APPROVER_TYPE_UNKNOWN = 'approval-approver-type-unknown';
34+
export const APPROVAL_ESCALATION_REASSIGN_NO_TARGET = 'approval-escalation-reassign-no-target';
3335

3436
export type ApprovalApproverSeverity = 'error' | 'warning' | 'info';
3537

@@ -136,6 +138,28 @@ export function validateApprovalApprovers(stack: AnyRec): ApprovalApproverFindin
136138
});
137139
}
138140
}
141+
142+
// escalation.action 'reassign' with no escalateTo silently degrades to a
143+
// plain SLA-breach notification at runtime — the hand-off the author
144+
// asked for never happens.
145+
const escalation = (cfg.escalation ?? null) as AnyRec | null;
146+
if (escalation && typeof escalation === 'object' && escalation.action === 'reassign') {
147+
const target = typeof escalation.escalateTo === 'string' ? escalation.escalateTo.trim() : '';
148+
if (!target) {
149+
findings.push({
150+
severity: 'warning',
151+
rule: APPROVAL_ESCALATION_REASSIGN_NO_TARGET,
152+
where,
153+
path: `flows[${fi}].nodes[${ni}].config.escalation.escalateTo`,
154+
message:
155+
`escalation.action is 'reassign' but escalateTo is empty — at runtime the ` +
156+
`escalation degrades to a notify and the request stays with the original approvers.`,
157+
hint:
158+
`Set escalateTo to a position machine name (expanded via sys_user_position, ` +
159+
`ADR-0090 D3) or a specific user id, or change action to 'notify'.`,
160+
});
161+
}
162+
}
139163
}
140164
}
141165

packages/plugins/plugin-approvals/src/approval-service.test.ts

Lines changed: 34 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -863,6 +863,40 @@ describe('ApprovalService (node era)', () => {
863863
expect(fresh?.pending_approvers).toEqual(['boss']);
864864
});
865865

866+
it('runEscalations: reassign expands a position escalateTo to its holders (ADR-0090 D3)', async () => {
867+
engine._tables['sys_user_position'] = [
868+
{ id: 'up1', user_id: 'u5', position: 'approvals_supervisor', organization_id: 't1' },
869+
{ id: 'up2', user_id: 'u6', position: 'approvals_supervisor', organization_id: 't1' },
870+
{ id: 'up3', user_id: 'u7', position: 'approvals_supervisor', organization_id: 't2' }, // other tenant
871+
];
872+
const req = await svc.openNodeRequest(
873+
openInput(['u9'], {}, { escalation: { timeoutHours: 1, action: 'reassign', escalateTo: 'approvals_supervisor', notifySubmitter: false } }), CTX,
874+
);
875+
makeOverdue(req.id);
876+
await svc.runEscalations();
877+
const fresh = await svc.getRequest(req.id, SYS);
878+
expect(fresh?.status).toBe('pending');
879+
expect(fresh?.pending_approvers?.slice().sort()).toEqual(['u5', 'u6']);
880+
// The audit trail keeps the AUTHORED target, not the expansion.
881+
const actions = await svc.listActions(req.id, SYS);
882+
expect(actions.find(a => a.action === 'escalate')?.comment).toBe('reassign → approvals_supervisor');
883+
});
884+
885+
it('runEscalations: notify expands a position escalateTo into the audience', async () => {
886+
engine._tables['sys_user_position'] = [
887+
{ id: 'up1', user_id: 'u5', position: 'approvals_supervisor', organization_id: 't1' },
888+
];
889+
const emitted: any[] = [];
890+
svc.attachMessaging({ async emit(input) { emitted.push(input); } });
891+
const req = await svc.openNodeRequest(
892+
openInput(['u9'], {}, { escalation: { timeoutHours: 2, action: 'notify', escalateTo: 'approvals_supervisor', notifySubmitter: false } }), CTX,
893+
);
894+
makeOverdue(req.id);
895+
await svc.runEscalations();
896+
expect(emitted).toHaveLength(1);
897+
expect(emitted[0].audience).toEqual(['u9', 'u5']);
898+
});
899+
866900
it('runEscalations: skips requests that are not yet due or have no SLA', async () => {
867901
await svc.openNodeRequest(
868902
openInput(['u9'], {}, { escalation: { timeoutHours: 1000, action: 'auto_approve' } }), CTX,

packages/plugins/plugin-approvals/src/approval-service.ts

Lines changed: 17 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -1381,6 +1381,18 @@ export class ApprovalService implements IApprovalService {
13811381
const now = this.clock.now().toISOString();
13821382
const pending = csvSplit(raw.pending_approvers);
13831383

1384+
// `escalateTo` is a position machine name or a user id (same contract as
1385+
// the `position` ApproverType, ADR-0090 D3). Position holders win; an
1386+
// empty expansion falls back to the literal, so a config naming a
1387+
// specific user id keeps working unchanged.
1388+
let escalatees: string[] = [];
1389+
if (escalateTo) {
1390+
try {
1391+
escalatees = await this.expandPositionUsers(escalateTo, raw.organization_id ?? null);
1392+
} catch { escalatees = []; }
1393+
if (!escalatees.length) escalatees = [escalateTo];
1394+
}
1395+
13841396
// Audit first — this row IS the idempotency marker (ADR-0042 §1).
13851397
await this.engine.insert('sys_approval_action', {
13861398
id: uid('aact'), request_id: raw.id, organization_id: raw.organization_id ?? null,
@@ -1390,14 +1402,14 @@ export class ApprovalService implements IApprovalService {
13901402
created_at: now,
13911403
}, { context: SYSTEM_CTX });
13921404

1393-
if (action === 'reassign' && escalateTo) {
1405+
if (action === 'reassign' && escalatees.length) {
13941406
await this.engine.update('sys_approval_request', {
1395-
id: raw.id, pending_approvers: escalateTo, updated_at: now,
1407+
id: raw.id, pending_approvers: escalatees.join(','), updated_at: now,
13961408
}, { context: SYSTEM_CTX });
1397-
await this.syncApproverIndex(raw.id, [escalateTo], raw.organization_id ?? null, now);
1409+
await this.syncApproverIndex(raw.id, escalatees, raw.organization_id ?? null, now);
13981410
await this.notify({
13991411
topic: 'approval.escalated',
1400-
audience: [escalateTo],
1412+
audience: escalatees,
14011413
actorId: SLA_ACTOR_ID,
14021414
source: { object: 'sys_approval_request', id: raw.id },
14031415
payload: {
@@ -1416,7 +1428,7 @@ export class ApprovalService implements IApprovalService {
14161428
// 'notify' (and the reassign-without-target fallback)
14171429
await this.notify({
14181430
topic: 'approval.sla_breached',
1419-
audience: [...pending, ...(escalateTo ? [escalateTo] : [])],
1431+
audience: [...pending, ...escalatees],
14201432
actorId: SLA_ACTOR_ID,
14211433
source: { object: 'sys_approval_request', id: raw.id },
14221434
payload: {

packages/spec/src/automation/approval.zod.ts

Lines changed: 8 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -119,12 +119,15 @@ export const ApprovalEscalationSchema = lazySchema(() => z.object({
119119
timeoutHours: z.number().min(1).describe('Hours before escalation triggers'),
120120
action: z.enum(['reassign', 'auto_approve', 'auto_reject', 'notify']).default('notify')
121121
.describe('Action on escalation timeout'),
122-
// Escalation hands the request to a role (the common case — e.g. a manager
123-
// role or an approvals queue owner); the Studio designer renders a role
124-
// picker, but free text is still accepted for a specific user id.
122+
// Escalation hands the request to a position (the common case — e.g. an
123+
// approvals supervisor); the Studio designer renders a position picker, but
124+
// free text is still accepted for a specific user id. The engine expands a
125+
// position machine name to its holders via `sys_user_position` (ADR-0090
126+
// D3) and falls back to treating the value as a user id when nobody holds
127+
// it. NOT a better-auth membership tier — same contract as ApproverType.
125128
escalateTo: z.string().optional().meta({
126-
description: 'User id, role, or manager level to escalate to',
127-
xRef: { kind: 'role' },
129+
description: 'User id or position machine name to escalate to',
130+
xRef: { kind: 'position' },
128131
}),
129132
notifySubmitter: z.boolean().default(true).describe('Notify the original submitter on escalation'),
130133
}));

skills/objectstack-automation/SKILL.md

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -392,7 +392,7 @@ branch — you never resume the flow by hand.
392392
| `behavior` | `first_response` (first approver decides) or `unanimous` (all must approve). Default `first_response` |
393393
| `lockRecord` | Lock the triggering record from edits while pending. Default `true` |
394394
| `approvalStatusField` | Business-object field to mirror `pending`/`approved`/`rejected`/`recalled` onto (should be readonly) |
395-
| `escalation` | Optional per-node SLA — `{ enabled, timeoutHours, action: reassign\|auto_approve\|auto_reject\|notify, escalateTo?, notifySubmitter }` |
395+
| `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) |
396396
| `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 |
397397

398398
### Branching, side-effects & rejection

0 commit comments

Comments
 (0)