Skip to content

Commit 474fe39

Browse files
authored
feat(spec+approvals+lint): approver value bindings, retire queue authoring (#3508) (#3536)
把「注册表 vs 数据记录」的分流规则写进 spec 契约:新增 APPROVER_VALUE_BINDINGS 逐类型声明 Value 的数据来源与存值(position 存机器名,user/team/department 存 行 id,department 指向 sys_business_unit),satisfies 保证与 ApproverType 枚举 完备对齐。 新增 NON_AUTHORABLE_APPROVER_TYPES(弃用拼写 + queue),经 xEnumDeprecated 发布 ——设计器下拉不再提供 queue,存量行仍解析、仍渲染。plugin-approvals 在解析存量 queue 审批人落死值前输出 warn;lint 新增 approval-approver-type-unsupported 规则,数据驱动自 APPROVER_VALUE_BINDINGS。
1 parent 3216344 commit 474fe39

10 files changed

Lines changed: 280 additions & 21 deletions

File tree

Lines changed: 26 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,26 @@
1+
---
2+
'@objectstack/spec': minor
3+
'@objectstack/plugin-approvals': patch
4+
'@objectstack/lint': minor
5+
---
6+
7+
feat(approvals): declare approver value bindings; retire `queue` approver authoring (#3508)
8+
9+
- `@objectstack/spec` exports `APPROVER_VALUE_BINDINGS` — the single declaration of how a
10+
designer must source each approver row's `value`: `user`/`team`/`department`/`position`
11+
are DATA-record lookups on the system directory objects (`sys_user` / `sys_team` /
12+
`sys_business_unit` / `sys_position`; `position` commits the machine **name**, the
13+
others the row id), `org_membership_level` is a closed enum (`ORG_MEMBERSHIP_LEVELS`),
14+
`manager` is auto-resolved, `field` names a trigger-object field, and `queue` is
15+
unsupported. Also exports `NON_AUTHORABLE_APPROVER_TYPES`.
16+
- `queue` approver type is deprecated-for-authoring: it still parses (stored flows keep
17+
loading and rendering) but is published in `xEnumDeprecated`, so designers stop
18+
offering it — the runtime has no queue resolution and the slot routes to nobody. The
19+
approver `value` xRef now also maps `manager`, so designers can render its
20+
auto-resolved state. No authored key is removed; nothing to migrate. If a flow carries
21+
`{ type: 'queue' }`, replace it with `team` / `department` / `position` (or a concrete
22+
`user`) until a real ownership-queue implementation lands.
23+
- `@objectstack/plugin-approvals` now warns at resolution time when a stored `queue`
24+
approver is skipped.
25+
- `@objectstack/lint` adds `approval-approver-type-unsupported` (warning) for approver
26+
types that are declared but not implemented by the runtime.

content/docs/automation/approvals.mdx

Lines changed: 5 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -214,11 +214,14 @@ Only `approvers` is required on the node; everything else has a default
214214
(`behavior: 'first_response'`, `lockRecord: true`, `maxRevisions: 3`).
215215

216216
Approver entries resolve by kind — `position`, `user`, `field`, `manager`,
217-
`team`, `department`, `queue`, `org_membership_level`, and `expression`
217+
`team`, `department`, `org_membership_level`, and `expression`
218218
(described in the [callout above](#3-the-approval-node) and in
219219
[Dynamic approvers](#dynamic-approvers-3447)); `org_membership_level` is the
220220
one that silently resolves to nobody when it's mistaken for a business
221-
hierarchy. `field`, `manager`, and `expression` resolve against the record's
221+
hierarchy. `queue` still parses so stored flows keep loading, but it is **not
222+
implemented** by the runtime and is no longer offered for authoring (#3508) —
223+
a queue entry resolves to nobody. Route to a `team`, `department`, or
224+
`position` instead. `field`, `manager`, and `expression` resolve against the record's
222225
**live** state at node entry (#3447). An entry that resolves
223226
to nobody is not an error: the request opens with an empty `pending_approvers`
224227
and nothing can move it, so the run parks forever.

content/docs/references/automation/approval.mdx

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -55,7 +55,7 @@ const result = ApprovalDecision.parse(data);
5555
| Property | Type | Required | Description |
5656
| :--- | :--- | :--- | :--- |
5757
| **type** | `Enum<'user' \| 'org_membership_level' \| 'role' \| 'position' \| 'team' \| 'department' \| 'manager' \| 'field' \| 'queue' \| 'expression'>` || |
58-
| **value** | `string` | optional | User id / membership tier / position / team / department / field / queue — per `type`; for `expression`, a CEL expression over `current.*` / `trigger.*` / `vars.*` |
58+
| **value** | `string` | optional | User id / membership tier / position / team / department / field — per `type`; for `expression`, a CEL expression over `current.*` / `trigger.*` / `vars.*` |
5959
| **resolveAs** | `Enum<'user' \| 'department' \| 'position' \| 'team'>` | optional | How an `expression` result is expanded into approvers (default 'user') |
6060
| **group** | `string` | optional | Group label for per_group sign-off (e.g. "legal", "finance") |
6161

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

Lines changed: 18 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -6,6 +6,7 @@ import {
66
APPROVAL_APPROVER_NOT_MEMBERSHIP_TIER,
77
APPROVAL_APPROVER_TYPE_DEPRECATED,
88
APPROVAL_APPROVER_TYPE_UNKNOWN,
9+
APPROVAL_APPROVER_TYPE_UNSUPPORTED,
910
APPROVAL_ESCALATION_REASSIGN_NO_TARGET,
1011
APPROVAL_APPROVERS_MAY_RESOLVE_EMPTY,
1112
APPROVAL_EXPRESSION_INVALID,
@@ -54,6 +55,20 @@ describe('validateApprovalApprovers', () => {
5455
expect(findings[0].hint).toContain("type: 'position'");
5556
});
5657

58+
// ── queue: declared-but-unenforced (#3508) ────────────────────────────────
59+
60+
it('flags a queue approver as unsupported — the runtime resolves it to nobody', () => {
61+
const findings = validateApprovalApprovers(stackWithApprovers([
62+
{ type: 'queue', value: 'q_west' },
63+
{ type: 'user', value: 'u1' }, // a real route keeps the node off the empty-slate rule
64+
]));
65+
expect(findings).toHaveLength(1);
66+
expect(findings[0].rule).toBe(APPROVAL_APPROVER_TYPE_UNSUPPORTED);
67+
expect(findings[0].severity).toBe('warning');
68+
expect(findings[0].path).toBe('flows[0].nodes[1].config.approvers[0].type');
69+
expect(findings[0].message).toContain('#3508');
70+
});
71+
5772
// ── the deprecated `role` spelling (ADR-0090 D3, #3133) ──────────────────
5873

5974
it('flags the deprecated `role` spelling even when its value is a valid tier', () => {
@@ -80,14 +95,15 @@ describe('validateApprovalApprovers', () => {
8095
expect(findings[0].hint).not.toContain('org_membership_level, value');
8196
});
8297

83-
it('accepts the position approver type and the other spec types silently', () => {
98+
it('accepts the position approver type and the other RESOLVABLE spec types silently', () => {
99+
// `queue` is deliberately absent: it parses but the runtime never resolves
100+
// it, so it now draws APPROVAL_APPROVER_TYPE_UNSUPPORTED (#3508).
84101
const findings = validateApprovalApprovers(stackWithApprovers([
85102
{ type: 'position', value: 'sales_manager' },
86103
{ type: 'user', value: 'u1' },
87104
{ type: 'manager' },
88105
{ type: 'department', value: 'bu_sales' },
89106
{ type: 'field', value: 'owner_id' },
90-
{ type: 'queue', value: 'q1' },
91107
{ type: 'team', value: 't1' },
92108
]));
93109
expect(findings).toEqual([]);

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

Lines changed: 21 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -41,6 +41,7 @@ import {
4141
ApproverType,
4242
APPROVAL_NODE_TYPE,
4343
DEPRECATED_APPROVER_TYPES,
44+
APPROVER_VALUE_BINDINGS,
4445
canonicalApproverType,
4546
normalizeDecisionOutputs,
4647
} from '@objectstack/spec/automation';
@@ -49,6 +50,7 @@ import { collectCelRootIdentifiers } from '@objectstack/formula';
4950
export const APPROVAL_APPROVER_NOT_MEMBERSHIP_TIER = 'approval-approver-not-membership-tier';
5051
export const APPROVAL_APPROVER_TYPE_DEPRECATED = 'approval-approver-type-deprecated';
5152
export const APPROVAL_APPROVER_TYPE_UNKNOWN = 'approval-approver-type-unknown';
53+
export const APPROVAL_APPROVER_TYPE_UNSUPPORTED = 'approval-approver-type-unsupported';
5254
export const APPROVAL_ESCALATION_REASSIGN_NO_TARGET = 'approval-escalation-reassign-no-target';
5355
export const APPROVAL_APPROVERS_MAY_RESOLVE_EMPTY = 'approval-approvers-may-resolve-empty';
5456
export const APPROVAL_EXPRESSION_INVALID = 'approval-expression-invalid';
@@ -270,6 +272,25 @@ export function validateApprovalApprovers(stack: AnyRec): ApprovalApproverFindin
270272
`is removed in the next major.`,
271273
hint: `Author { type: '${fix}', value: '${value}' }. It resolves identically today.`,
272274
});
275+
} else if (
276+
(APPROVER_VALUE_BINDINGS as Record<string, { source: string }>)[canonical]?.source === 'unsupported'
277+
) {
278+
// Declared-but-unenforced (#3508): the runtime has no resolution for
279+
// this type — the slot degrades to an inert `type:value` literal and
280+
// the request routes to nobody. Say it at authoring time instead of
281+
// letting the request stall silently (Prime Directive #10).
282+
findings.push({
283+
severity: 'warning',
284+
rule: APPROVAL_APPROVER_TYPE_UNSUPPORTED,
285+
where,
286+
path: `${path}.type`,
287+
message:
288+
`approver type '${type}' is declared but not implemented by the runtime (#3508) — ` +
289+
`the slot resolves to nobody and the request stalls.`,
290+
hint:
291+
`Route to people the engine can expand: { type: 'team' | 'department' | 'position', ... }. ` +
292+
`Queue approvers need a real ownership-queue implementation before they take effect.`,
293+
});
273294
}
274295
}
275296

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

Lines changed: 25 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1980,3 +1980,28 @@ describe('ApprovalService — listActions attachments mapping (#3266)', () => {
19801980
expect(approve?.attachments).toEqual(['file_a']);
19811981
});
19821982
});
1983+
1984+
// #3508: `queue` is declared-but-unenforced — resolveApproverSpec has no queue
1985+
// branch, so the spec value falls through to the dead `queue:<id>` literal.
1986+
// The engine must at least WARN so operators can see the silent dead slot;
1987+
// the spec marks the type non-authorable so designers stop offering it.
1988+
describe('ApprovalService — queue approver is unresolved (#3508)', () => {
1989+
it('falls back to the dead literal and warns', async () => {
1990+
const engine = makeFakeEngine();
1991+
const warnings: any[] = [];
1992+
let n = 0;
1993+
const svc = new ApprovalService({
1994+
engine: engine as any,
1995+
clock: { now: () => new Date(1757000000000 + (n++) * 1000) },
1996+
logger: { warn: (msg: any, meta: any) => warnings.push([msg, meta]) },
1997+
});
1998+
const req = await svc.openNodeRequest(
1999+
{ ...openInput([]), config: { approvers: [{ type: 'queue', value: 'q_west' }], behavior: 'first_response', lockRecord: false } },
2000+
CTX,
2001+
);
2002+
// No queue expansion exists: the slot is the raw `type:value` literal,
2003+
// which matches no real user id — the request routes to nobody.
2004+
expect(req.pending_approvers).toEqual(['queue:q_west']);
2005+
expect(warnings.some(([msg]) => String(msg).includes("'queue'") && String(msg).includes('#3508'))).toBe(true);
2006+
});
2007+
});

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

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -561,6 +561,18 @@ export class ApprovalService implements IApprovalService {
561561
}
562562
}
563563
} catch { /* fall through */ }
564+
// #3508: `queue` is declared-but-unenforced — there is no queue branch
565+
// above, so a queue approver always lands here and the `queue:<id>` slot
566+
// routes to nobody. The spec marks it non-authorable
567+
// (NON_AUTHORABLE_APPROVER_TYPES) so designers stop offering it; warn for
568+
// the stored flows that still carry one, so the silent dead slot is at
569+
// least visible to operators.
570+
if (type === 'queue') {
571+
this.logger?.warn?.(
572+
`[approvals] approver type 'queue' is not implemented — the slot resolves to nobody (#3508)`,
573+
{ value: a.value },
574+
);
575+
}
564576
return [`${a.type}:${a.value}`];
565577
}
566578

packages/spec/api-surface.json

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1975,6 +1975,8 @@
19751975
"./automation": [
19761976
"APPROVAL_BRANCH_LABELS (const)",
19771977
"APPROVAL_NODE_TYPE (const)",
1978+
"APPROVER_EXPRESSION_ROOTS (const)",
1979+
"APPROVER_VALUE_BINDINGS (const)",
19781980
"ActionCategory (type)",
19791981
"ActionCategorySchema (const)",
19801982
"ActionDescriptor (type)",
@@ -1992,6 +1994,7 @@
19921994
"ApprovalNodeConfig (type)",
19931995
"ApprovalNodeConfigSchema (const)",
19941996
"ApproverType (const)",
1997+
"ApproverValueBinding (type)",
19951998
"AuthField (type)",
19961999
"AuthFieldSchema (const)",
19972000
"Authentication (type)",
@@ -2102,10 +2105,12 @@
21022105
"LoopConfigParsed (type)",
21032106
"LoopConfigSchema (const)",
21042107
"MappableFlow (interface)",
2108+
"NON_AUTHORABLE_APPROVER_TYPES (const)",
21052109
"NodeExecutorDescriptor (type)",
21062110
"NodeExecutorDescriptorSchema (const)",
21072111
"OAuth2Config (type)",
21082112
"OAuth2ConfigSchema (const)",
2113+
"ORG_MEMBERSHIP_LEVELS (const)",
21092114
"OS_CONSTRUCT_EXT (const)",
21102115
"OperationParameter (type)",
21112116
"OperationParameterSchema (const)",

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

Lines changed: 63 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,9 @@ import { describe, it, expect } from 'vitest';
22
import {
33
ApproverType,
44
DEPRECATED_APPROVER_TYPES,
5+
NON_AUTHORABLE_APPROVER_TYPES,
6+
APPROVER_VALUE_BINDINGS,
7+
ORG_MEMBERSHIP_LEVELS,
58
canonicalApproverType,
69
APPROVAL_NODE_TYPE,
710
ApprovalDecision,
@@ -45,15 +48,71 @@ describe('ApproverType', () => {
4548
// Cross-repo contract: the published node configSchema must carry
4649
// `xEnumDeprecated` on the approver type, or the Studio designer (objectui)
4750
// derives its dropdown straight from `enum` and keeps offering `role` — the
48-
// exact trap ADR-0090 D3 retires. Renderers read this to omit deprecated
49-
// members from pickers while still rendering a stored value.
50-
it('publishes xEnumDeprecated on the approver type so pickers can drop `role`', () => {
51+
// exact trap ADR-0090 D3 retires — and `queue`, which the runtime never
52+
// resolves (#3508). Renderers read this to omit these members from pickers
53+
// while still rendering a stored value.
54+
it('publishes xEnumDeprecated on the approver type so pickers drop `role` and `queue`', () => {
5155
const schema = getApprovalNodeConfigJsonSchema() as any;
5256
const typeNode = schema?.properties?.approvers?.items?.properties?.type;
5357
expect(typeNode?.enum).toContain('role'); // still parses (back-compat)
58+
expect(typeNode?.enum).toContain('queue'); // still parses (stored rows render)
5459
expect(typeNode?.enum).toContain('org_membership_level');
55-
expect(typeNode?.xEnumDeprecated).toEqual(Object.keys(DEPRECATED_APPROVER_TYPES));
60+
expect(typeNode?.xEnumDeprecated).toEqual([...NON_AUTHORABLE_APPROVER_TYPES]);
5661
expect(typeNode?.xEnumDeprecated).toContain('role');
62+
expect(typeNode?.xEnumDeprecated).toContain('queue');
63+
});
64+
65+
// #3508: queue is declared-but-unenforced. It stays parseable (stored flows
66+
// keep loading) but is not authorable, and every deprecated spelling is
67+
// non-authorable too.
68+
it('marks queue non-authorable while keeping it parseable', () => {
69+
expect(() => ApproverType.parse('queue')).not.toThrow();
70+
expect(NON_AUTHORABLE_APPROVER_TYPES).toContain('queue');
71+
for (const spelling of Object.keys(DEPRECATED_APPROVER_TYPES)) {
72+
expect(NON_AUTHORABLE_APPROVER_TYPES).toContain(spelling);
73+
}
74+
});
75+
});
76+
77+
// #3508: the designer contract for sourcing each approver row's `value`. The
78+
// record-backed kinds MUST match the engine's resolution semantics
79+
// (`plugin-approvals` resolveApproverSpec / expand*Users) — these assertions
80+
// pin the object names and stored fields the engine actually queries.
81+
describe('APPROVER_VALUE_BINDINGS (#3508)', () => {
82+
it('covers every ApproverType member', () => {
83+
for (const t of ApproverType.options) {
84+
expect(APPROVER_VALUE_BINDINGS[t]).toBeDefined();
85+
}
86+
});
87+
88+
it('binds record-backed kinds to the objects and fields the engine queries', () => {
89+
// applyOooDelegation(String(value)) — a sys_user id.
90+
expect(APPROVER_VALUE_BINDINGS.user).toEqual({ source: 'record', object: 'sys_user', valueField: 'id' });
91+
// find('sys_team_member', { team_id: value }) — a sys_team id.
92+
expect(APPROVER_VALUE_BINDINGS.team).toEqual({ source: 'record', object: 'sys_team', valueField: 'id' });
93+
// find('sys_business_unit', { id: value }) — a sys_business_unit id
94+
// (deliberately NOT a `sys_department`).
95+
expect(APPROVER_VALUE_BINDINGS.department).toEqual({ source: 'record', object: 'sys_business_unit', valueField: 'id' });
96+
// find('sys_user_position', { position: value }) — the position machine
97+
// NAME, not an id (portable across environments).
98+
expect(APPROVER_VALUE_BINDINGS.position).toEqual({ source: 'record', object: 'sys_position', valueField: 'name' });
99+
});
100+
101+
it('keeps the non-record kinds off the record-lookup path', () => {
102+
expect(APPROVER_VALUE_BINDINGS.org_membership_level).toEqual({ source: 'enum', values: ORG_MEMBERSHIP_LEVELS });
103+
// The deprecated alias renders with the same control as its replacement.
104+
expect(APPROVER_VALUE_BINDINGS.role).toEqual(APPROVER_VALUE_BINDINGS.org_membership_level);
105+
expect(APPROVER_VALUE_BINDINGS.manager.source).toBe('auto');
106+
expect(APPROVER_VALUE_BINDINGS.field.source).toBe('trigger-field');
107+
expect(APPROVER_VALUE_BINDINGS.queue.source).toBe('unsupported');
108+
});
109+
110+
it('publishes a manager mapping in the value xRef so designers can render auto-resolution', () => {
111+
const schema = getApprovalNodeConfigJsonSchema() as any;
112+
const valueNode = schema?.properties?.approvers?.items?.properties?.value;
113+
expect(valueNode?.xRef?.map?.manager).toBe('manager');
114+
// queue stays mapped so stored rows keep rendering during the window.
115+
expect(valueNode?.xRef?.map?.queue).toBe('queue');
57116
});
58117
});
59118

0 commit comments

Comments
 (0)