Skip to content

Commit 57bab76

Browse files
os-zhuangclaude
andauthored
feat(approvals): typed decisionOutputs — record pickers for decision-output fields (#3447) (#3554)
A decisionOutputs entry may be { key, label?, type, multiple? } alongside the bare string: typed entries render sys_user / business-unit / position / team pickers in the decision dialog (multiple → id array) instead of free text. normalizeDecisionOutputs (spec) is the one reader of the union — service whitelist, request read and os lint all key off it, so bare and typed can never behave differently. The request read adds decision_output_defs next to the skew-safe decision_outputs key list; the showcase demo flow and SKILL.md adopt the typed form, and the end-to-end skill example is now os:check'd (compiled against the spec in CI). Co-authored-by: Jack Zhuang <277994282+os-zhuang@users.noreply.github.com> Co-authored-by: Claude Fable 5 <noreply@anthropic.com>
1 parent d8c4957 commit 57bab76

13 files changed

Lines changed: 218 additions & 29 deletions

File tree

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,7 @@
1+
---
2+
"@objectstack/spec": minor
3+
"@objectstack/plugin-approvals": minor
4+
"@objectstack/lint": patch
5+
---
6+
7+
Typed `decisionOutputs` declarations (#3447 follow-up). A `decisionOutputs` entry may now be `{ key, label?, type: 'text' | 'user' | 'department' | 'position' | 'team', multiple? }` alongside the bare-string form — a typed entry tells the decision UI to render the matching record picker (id values; `multiple` collects an id array) instead of free text, turning "paste user ids" into "pick people". The type shapes only the input widget: the runtime whitelist works by `key` either way, via the new `normalizeDecisionOutputs` helper exported from `@objectstack/spec/automation` — the single reader of the union shape shared by the service, the request read, and `os lint`. The request read now carries `decision_output_defs` (normalized declarations) alongside the version-skew-safe `decision_outputs` key list.

content/docs/automation/approvals.mdx

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -188,6 +188,13 @@ node B's approver be
188188
declares output keys, approvers only fill values; undeclared keys reject the
189189
decision, and `decision` / `requestId` are reserved.
190190

191+
A declaration can also be **typed** — `{ key: 'next_reviewers', type: 'user',
192+
multiple: true }` — which renders a multi-select user picker in the decision
193+
dialog instead of free text (`department` / `position` / `team` render the
194+
matching system-object picker; picker values are record ids, `multiple`
195+
collects an id array). The type shapes only the input widget; the whitelist
196+
still works by `key`, so bare strings and typed declarations mix freely.
197+
191198
## The full lifecycle — submit to field change
192199

193200
What actually happens between "the flow hits the approval node" and "the record

content/docs/references/automation/approval.mdx

Lines changed: 17 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -14,8 +14,8 @@ Approval Step Approver Type
1414
## TypeScript Usage
1515

1616
```typescript
17-
import { ApprovalDecision, ApprovalEscalation, ApprovalNodeApprover, ApprovalNodeConfig, ApproverType } from '@objectstack/spec/automation';
18-
import type { ApprovalDecision, ApprovalEscalation, ApprovalNodeApprover, ApprovalNodeConfig, ApproverType } from '@objectstack/spec/automation';
17+
import { ApprovalDecision, ApprovalEscalation, ApprovalNodeApprover, ApprovalNodeConfig, ApproverType, DecisionOutputDef } from '@objectstack/spec/automation';
18+
import type { ApprovalDecision, ApprovalEscalation, ApprovalNodeApprover, ApprovalNodeConfig, ApproverType, DecisionOutputDef } from '@objectstack/spec/automation';
1919

2020
// Validate data
2121
const result = ApprovalDecision.parse(data);
@@ -74,7 +74,7 @@ const result = ApprovalDecision.parse(data);
7474
| **lockRecord** | `boolean` || Lock the record from editing while pending |
7575
| **approvalStatusField** | `string` | optional | Business-object field to mirror request status onto |
7676
| **onEmptyApprovers** | `Enum<'admin_rescue' \| 'fail' \| 'auto_approve'>` || Behavior when no concrete approver resolves at node entry |
77-
| **decisionOutputs** | `string[]` | optional | Author-declared output keys a decision may carry (approvers fill values only) |
77+
| **decisionOutputs** | `string \| { key: string; label?: string; type?: Enum<'text' \| 'user' \| 'department' \| 'position' \| 'team'>; multiple?: boolean }[]` | optional | Author-declared decision outputs — bare keys or typed `{ key, type, multiple }` declarations |
7878
| **escalation** | `{ enabled: boolean; timeoutHours: number; action: Enum<'reassign' \| 'auto_approve' \| 'auto_reject' \| 'notify'>; escalateTo?: string; … }` | optional | Per-node SLA escalation |
7979
| **maxRevisions** | `integer` || Max send-backs for revision before auto-reject (0 = send-back disabled) |
8080

@@ -99,3 +99,17 @@ const result = ApprovalDecision.parse(data);
9999

100100
---
101101

102+
## DecisionOutputDef
103+
104+
### Properties
105+
106+
| Property | Type | Required | Description |
107+
| :--- | :--- | :--- | :--- |
108+
| **key** | `string` || Output key (the flow variable name under the node id) |
109+
| **label** | `string` | optional | Field label in the decision dialog |
110+
| **type** | `Enum<'text' \| 'user' \| 'department' \| 'position' \| 'team'>` | optional | Decision-dialog input widget (default 'text') |
111+
| **multiple** | `boolean` | optional | Collect multiple values (id array) |
112+
113+
114+
---
115+

examples/app-showcase/src/automation/flows/dynamic-approval.flow.ts

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -39,8 +39,10 @@ export const DynamicApprovalFlow = defineFlow({
3939
approvers: [{ type: 'org_membership_level', value: 'owner' }],
4040
behavior: 'first_response',
4141
lockRecord: true,
42-
// The lead hands the co-signers to the flow with their decision.
43-
decisionOutputs: ['next_reviewers'],
42+
// The lead hands the co-signers to the flow with their decision. The
43+
// TYPED declaration renders a multi-select sys_user picker in the
44+
// decision dialog (bare string keys render free text).
45+
decisionOutputs: [{ key: 'next_reviewers', label: 'Next Reviewers', type: 'user', multiple: true }],
4446
},
4547
},
4648
{

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

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -289,6 +289,15 @@ describe('expression approvers (#3447 P2)', () => {
289289
expect(findings[0].message).toContain('decision');
290290
});
291291

292+
it('errors on reserved keys inside TYPED decisionOutputs declarations too', () => {
293+
const findings = validateApprovalApprovers(stackWithConfig({
294+
approvers: [{ type: 'user', value: 'u1' }],
295+
decisionOutputs: [{ key: 'decision', type: 'user' }, { key: 'ok' }],
296+
}));
297+
expect(findings).toHaveLength(1);
298+
expect(findings[0].rule).toBe(APPROVAL_DECISION_OUTPUTS_RESERVED);
299+
});
300+
292301
it('accepts declared non-reserved decisionOutputs', () => {
293302
expect(validateApprovalApprovers(stackWithConfig({
294303
approvers: [{ type: 'user', value: 'u1' }],

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

Lines changed: 4 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -42,6 +42,7 @@ import {
4242
APPROVAL_NODE_TYPE,
4343
DEPRECATED_APPROVER_TYPES,
4444
canonicalApproverType,
45+
normalizeDecisionOutputs,
4546
} from '@objectstack/spec/automation';
4647
import { collectCelRootIdentifiers } from '@objectstack/formula';
4748

@@ -332,9 +333,9 @@ export function validateApprovalApprovers(stack: AnyRec): ApprovalApproverFindin
332333
// #3447 P2: `decision`/`requestId` ride the resume envelope; a declared
333334
// decision output with either name is rejected at runtime on every
334335
// decide — the node can never accept the output it declares.
335-
const declaredOutputs = Array.isArray((cfg as AnyRec).decisionOutputs)
336-
? ((cfg as AnyRec).decisionOutputs as unknown[]).map(String)
337-
: [];
336+
// Bare keys and typed { key, … } declarations whitelist identically —
337+
// the spec normalizer is the one reader of the union shape.
338+
const declaredOutputs = normalizeDecisionOutputs((cfg as AnyRec).decisionOutputs).map((d) => d.key);
338339
const reserved = declaredOutputs.filter((k) => RESERVED_OUTPUT_KEYS.has(k));
339340
if (reserved.length) {
340341
findings.push({

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

Lines changed: 22 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -441,6 +441,28 @@ describe('ApprovalService (node era)', () => {
441441
}, SYS)).rejects.toThrow(/VALIDATION_FAILED.*reserved/s);
442442
});
443443

444+
it('decision outputs: typed declarations normalize — whitelist by key, defs surfaced for the UI', async () => {
445+
const req = await svc.openNodeRequest(
446+
openInput(['u9'], {}, {
447+
decisionOutputs: [
448+
{ key: 'next_reviewers', label: 'Next Reviewers', type: 'user', multiple: true },
449+
'note',
450+
],
451+
}), CTX,
452+
) as any;
453+
// Key list stays the version-skew-safe shape; defs carry the typed form.
454+
expect(req.decision_outputs).toEqual(['next_reviewers', 'note']);
455+
expect(req.decision_output_defs).toEqual([
456+
{ key: 'next_reviewers', label: 'Next Reviewers', type: 'user', multiple: true },
457+
{ key: 'note' },
458+
]);
459+
// A typed declaration whitelists exactly like a bare key.
460+
const out = await svc.decideNode(req.id, {
461+
decision: 'approve', actorId: 'u9', outputs: { next_reviewers: ['u2', 'u3'] },
462+
}, SYS);
463+
expect(out.outputs).toEqual({ next_reviewers: ['u2', 'u3'] });
464+
});
465+
444466
it('decision outputs: declared keys surface on the request row as decision_outputs (#3447 P2 UI)', async () => {
445467
// The decision UI renders one input per declared key — the keys ride the
446468
// request read (per-request; the static action params can't carry them).

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

Lines changed: 15 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -4,6 +4,7 @@ import { createHash, randomBytes } from 'node:crypto';
44
import {
55
APPROVAL_BRANCH_LABELS,
66
canonicalApproverType,
7+
normalizeDecisionOutputs,
78
type ApprovalNodeConfig,
89
} from '@objectstack/spec/automation';
910
import { ExpressionEngine, collectCelRootIdentifiers } from '@objectstack/formula';
@@ -229,13 +230,19 @@ function rowFromRequest(row: any): ApprovalRequestRow {
229230
sla_due_at: slaDueAt(row.created_at, cfg),
230231
// ADR-0044 revision round (rides the config snapshot; absent ⇒ round 1).
231232
round: typeof cfg?.__round === 'number' ? cfg.__round : undefined,
232-
// #3447 P2: the node's author-declared decision-output keys, surfaced so a
233+
// #3447 P2: the node's author-declared decision outputs, surfaced so a
233234
// decision UI can render input fields for them and POST `outputs` on
234235
// approve/reject. Per-request (each node declares its own), which is why
235-
// this rides the row instead of the static action params.
236-
decision_outputs: Array.isArray(cfg?.decisionOutputs) && cfg.decisionOutputs.length
237-
? cfg.decisionOutputs.map(String)
238-
: undefined,
236+
// this rides the row instead of the static action params. Two shapes for
237+
// version skew: `decision_outputs` stays the bare KEY list an older
238+
// console renders as text inputs; `decision_output_defs` carries the
239+
// normalized typed declarations a picker-aware console prefers.
240+
...(() => {
241+
const defs = normalizeDecisionOutputs(cfg?.decisionOutputs);
242+
return defs.length
243+
? { decision_outputs: defs.map(d => d.key), decision_output_defs: defs }
244+
: {};
245+
})(),
239246
} as any;
240247
}
241248

@@ -1220,9 +1227,9 @@ export class ApprovalService implements IApprovalService {
12201227
const outputKeys = input.outputs ? Object.keys(input.outputs) : [];
12211228
let acceptedOutputs: Record<string, unknown> | undefined;
12221229
if (outputKeys.length) {
1223-
const declared = Array.isArray((config as any).decisionOutputs)
1224-
? ((config as any).decisionOutputs as unknown[]).map(String)
1225-
: [];
1230+
// Typed declarations and bare keys whitelist identically — one
1231+
// normalizer (spec) is the single reader of the union shape.
1232+
const declared = normalizeDecisionOutputs((config as any).decisionOutputs).map(d => d.key);
12261233
if (!declared.length) {
12271234
throw new Error(
12281235
`VALIDATION_FAILED: this approval node declares no decisionOutputs — outputs are not accepted. `

packages/spec/api-surface.json

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2040,6 +2040,8 @@
20402040
"DataSourceConfigSchema (const)",
20412041
"DataSyncConfig (type)",
20422042
"DataSyncConfigSchema (const)",
2043+
"DecisionOutputDef (type)",
2044+
"DecisionOutputDefSchema (const)",
20432045
"ETL (const)",
20442046
"ETLDestination (type)",
20452047
"ETLDestinationSchema (const)",
@@ -2165,6 +2167,7 @@
21652167
"flowForm (const)",
21662168
"getApprovalNodeConfigJsonSchema (function)",
21672169
"importBpmnToConstructs (function)",
2170+
"normalizeDecisionOutputs (function)",
21682171
"validateControlFlow (function)"
21692172
],
21702173
"./api": [

packages/spec/json-schema.manifest.json

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -538,6 +538,7 @@
538538
"automation/DataDestinationConfig",
539539
"automation/DataSourceConfig",
540540
"automation/DataSyncConfig",
541+
"automation/DecisionOutputDef",
541542
"automation/ETLDestination",
542543
"automation/ETLEndpointType",
543544
"automation/ETLPipeline",

0 commit comments

Comments
 (0)