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
7 changes: 7 additions & 0 deletions .changeset/typed-decision-outputs-3447.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
---
"@objectstack/spec": minor
"@objectstack/plugin-approvals": minor
"@objectstack/lint": patch
---

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.
7 changes: 7 additions & 0 deletions content/docs/automation/approvals.mdx
Original file line number Diff line number Diff line change
Expand Up @@ -188,6 +188,13 @@ node B's approver be
declares output keys, approvers only fill values; undeclared keys reject the
decision, and `decision` / `requestId` are reserved.

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

## The full lifecycle — submit to field change

What actually happens between "the flow hits the approval node" and "the record
Expand Down
20 changes: 17 additions & 3 deletions content/docs/references/automation/approval.mdx
Original file line number Diff line number Diff line change
Expand Up @@ -14,8 +14,8 @@ Approval Step Approver Type
## TypeScript Usage

```typescript
import { ApprovalDecision, ApprovalEscalation, ApprovalNodeApprover, ApprovalNodeConfig, ApproverType } from '@objectstack/spec/automation';
import type { ApprovalDecision, ApprovalEscalation, ApprovalNodeApprover, ApprovalNodeConfig, ApproverType } from '@objectstack/spec/automation';
import { ApprovalDecision, ApprovalEscalation, ApprovalNodeApprover, ApprovalNodeConfig, ApproverType, DecisionOutputDef } from '@objectstack/spec/automation';
import type { ApprovalDecision, ApprovalEscalation, ApprovalNodeApprover, ApprovalNodeConfig, ApproverType, DecisionOutputDef } from '@objectstack/spec/automation';

// Validate data
const result = ApprovalDecision.parse(data);
Expand Down Expand Up @@ -74,7 +74,7 @@ const result = ApprovalDecision.parse(data);
| **lockRecord** | `boolean` | ✅ | Lock the record from editing while pending |
| **approvalStatusField** | `string` | optional | Business-object field to mirror request status onto |
| **onEmptyApprovers** | `Enum<'admin_rescue' \| 'fail' \| 'auto_approve'>` | ✅ | Behavior when no concrete approver resolves at node entry |
| **decisionOutputs** | `string[]` | optional | Author-declared output keys a decision may carry (approvers fill values only) |
| **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 |
| **escalation** | `{ enabled: boolean; timeoutHours: number; action: Enum<'reassign' \| 'auto_approve' \| 'auto_reject' \| 'notify'>; escalateTo?: string; … }` | optional | Per-node SLA escalation |
| **maxRevisions** | `integer` | ✅ | Max send-backs for revision before auto-reject (0 = send-back disabled) |

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

---

## DecisionOutputDef

### Properties

| Property | Type | Required | Description |
| :--- | :--- | :--- | :--- |
| **key** | `string` | ✅ | Output key (the flow variable name under the node id) |
| **label** | `string` | optional | Field label in the decision dialog |
| **type** | `Enum<'text' \| 'user' \| 'department' \| 'position' \| 'team'>` | optional | Decision-dialog input widget (default 'text') |
| **multiple** | `boolean` | optional | Collect multiple values (id array) |


---

Original file line number Diff line number Diff line change
Expand Up @@ -39,8 +39,10 @@ export const DynamicApprovalFlow = defineFlow({
approvers: [{ type: 'org_membership_level', value: 'owner' }],
behavior: 'first_response',
lockRecord: true,
// The lead hands the co-signers to the flow with their decision.
decisionOutputs: ['next_reviewers'],
// The lead hands the co-signers to the flow with their decision. The
// TYPED declaration renders a multi-select sys_user picker in the
// decision dialog (bare string keys render free text).
decisionOutputs: [{ key: 'next_reviewers', label: 'Next Reviewers', type: 'user', multiple: true }],
},
},
{
Expand Down
9 changes: 9 additions & 0 deletions packages/lint/src/validate-approval-approvers.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -289,6 +289,15 @@ describe('expression approvers (#3447 P2)', () => {
expect(findings[0].message).toContain('decision');
});

it('errors on reserved keys inside TYPED decisionOutputs declarations too', () => {
const findings = validateApprovalApprovers(stackWithConfig({
approvers: [{ type: 'user', value: 'u1' }],
decisionOutputs: [{ key: 'decision', type: 'user' }, { key: 'ok' }],
}));
expect(findings).toHaveLength(1);
expect(findings[0].rule).toBe(APPROVAL_DECISION_OUTPUTS_RESERVED);
});

it('accepts declared non-reserved decisionOutputs', () => {
expect(validateApprovalApprovers(stackWithConfig({
approvers: [{ type: 'user', value: 'u1' }],
Expand Down
7 changes: 4 additions & 3 deletions packages/lint/src/validate-approval-approvers.ts
Original file line number Diff line number Diff line change
Expand Up @@ -42,6 +42,7 @@ import {
APPROVAL_NODE_TYPE,
DEPRECATED_APPROVER_TYPES,
canonicalApproverType,
normalizeDecisionOutputs,
} from '@objectstack/spec/automation';
import { collectCelRootIdentifiers } from '@objectstack/formula';

Expand Down Expand Up @@ -332,9 +333,9 @@ export function validateApprovalApprovers(stack: AnyRec): ApprovalApproverFindin
// #3447 P2: `decision`/`requestId` ride the resume envelope; a declared
// decision output with either name is rejected at runtime on every
// decide — the node can never accept the output it declares.
const declaredOutputs = Array.isArray((cfg as AnyRec).decisionOutputs)
? ((cfg as AnyRec).decisionOutputs as unknown[]).map(String)
: [];
// Bare keys and typed { key, … } declarations whitelist identically —
// the spec normalizer is the one reader of the union shape.
const declaredOutputs = normalizeDecisionOutputs((cfg as AnyRec).decisionOutputs).map((d) => d.key);
const reserved = declaredOutputs.filter((k) => RESERVED_OUTPUT_KEYS.has(k));
if (reserved.length) {
findings.push({
Expand Down
22 changes: 22 additions & 0 deletions packages/plugins/plugin-approvals/src/approval-service.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -441,6 +441,28 @@ describe('ApprovalService (node era)', () => {
}, SYS)).rejects.toThrow(/VALIDATION_FAILED.*reserved/s);
});

it('decision outputs: typed declarations normalize — whitelist by key, defs surfaced for the UI', async () => {
const req = await svc.openNodeRequest(
openInput(['u9'], {}, {
decisionOutputs: [
{ key: 'next_reviewers', label: 'Next Reviewers', type: 'user', multiple: true },
'note',
],
}), CTX,
) as any;
// Key list stays the version-skew-safe shape; defs carry the typed form.
expect(req.decision_outputs).toEqual(['next_reviewers', 'note']);
expect(req.decision_output_defs).toEqual([
{ key: 'next_reviewers', label: 'Next Reviewers', type: 'user', multiple: true },
{ key: 'note' },
]);
// A typed declaration whitelists exactly like a bare key.
const out = await svc.decideNode(req.id, {
decision: 'approve', actorId: 'u9', outputs: { next_reviewers: ['u2', 'u3'] },
}, SYS);
expect(out.outputs).toEqual({ next_reviewers: ['u2', 'u3'] });
});

it('decision outputs: declared keys surface on the request row as decision_outputs (#3447 P2 UI)', async () => {
// The decision UI renders one input per declared key — the keys ride the
// request read (per-request; the static action params can't carry them).
Expand Down
23 changes: 15 additions & 8 deletions packages/plugins/plugin-approvals/src/approval-service.ts
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@ import { createHash, randomBytes } from 'node:crypto';
import {
APPROVAL_BRANCH_LABELS,
canonicalApproverType,
normalizeDecisionOutputs,
type ApprovalNodeConfig,
} from '@objectstack/spec/automation';
import { ExpressionEngine, collectCelRootIdentifiers } from '@objectstack/formula';
Expand Down Expand Up @@ -229,13 +230,19 @@ function rowFromRequest(row: any): ApprovalRequestRow {
sla_due_at: slaDueAt(row.created_at, cfg),
// ADR-0044 revision round (rides the config snapshot; absent ⇒ round 1).
round: typeof cfg?.__round === 'number' ? cfg.__round : undefined,
// #3447 P2: the node's author-declared decision-output keys, surfaced so a
// #3447 P2: the node's author-declared decision outputs, surfaced so a
// decision UI can render input fields for them and POST `outputs` on
// approve/reject. Per-request (each node declares its own), which is why
// this rides the row instead of the static action params.
decision_outputs: Array.isArray(cfg?.decisionOutputs) && cfg.decisionOutputs.length
? cfg.decisionOutputs.map(String)
: undefined,
// this rides the row instead of the static action params. Two shapes for
// version skew: `decision_outputs` stays the bare KEY list an older
// console renders as text inputs; `decision_output_defs` carries the
// normalized typed declarations a picker-aware console prefers.
...(() => {
const defs = normalizeDecisionOutputs(cfg?.decisionOutputs);
return defs.length
? { decision_outputs: defs.map(d => d.key), decision_output_defs: defs }
: {};
})(),
} as any;
}

Expand Down Expand Up @@ -1220,9 +1227,9 @@ export class ApprovalService implements IApprovalService {
const outputKeys = input.outputs ? Object.keys(input.outputs) : [];
let acceptedOutputs: Record<string, unknown> | undefined;
if (outputKeys.length) {
const declared = Array.isArray((config as any).decisionOutputs)
? ((config as any).decisionOutputs as unknown[]).map(String)
: [];
// Typed declarations and bare keys whitelist identically — one
// normalizer (spec) is the single reader of the union shape.
const declared = normalizeDecisionOutputs((config as any).decisionOutputs).map(d => d.key);
if (!declared.length) {
throw new Error(
`VALIDATION_FAILED: this approval node declares no decisionOutputs — outputs are not accepted. `
Expand Down
3 changes: 3 additions & 0 deletions packages/spec/api-surface.json
Original file line number Diff line number Diff line change
Expand Up @@ -2040,6 +2040,8 @@
"DataSourceConfigSchema (const)",
"DataSyncConfig (type)",
"DataSyncConfigSchema (const)",
"DecisionOutputDef (type)",
"DecisionOutputDefSchema (const)",
"ETL (const)",
"ETLDestination (type)",
"ETLDestinationSchema (const)",
Expand Down Expand Up @@ -2165,6 +2167,7 @@
"flowForm (const)",
"getApprovalNodeConfigJsonSchema (function)",
"importBpmnToConstructs (function)",
"normalizeDecisionOutputs (function)",
"validateControlFlow (function)"
],
"./api": [
Expand Down
1 change: 1 addition & 0 deletions packages/spec/json-schema.manifest.json
Original file line number Diff line number Diff line change
Expand Up @@ -538,6 +538,7 @@
"automation/DataDestinationConfig",
"automation/DataSourceConfig",
"automation/DataSyncConfig",
"automation/DecisionOutputDef",
"automation/ETLDestination",
"automation/ETLEndpointType",
"automation/ETLPipeline",
Expand Down
68 changes: 66 additions & 2 deletions packages/spec/src/automation/approval.zod.ts
Original file line number Diff line number Diff line change
Expand Up @@ -180,6 +180,58 @@ export const ApprovalNodeApproverSchema = lazySchema(() => z.object({
}));
export type ApprovalNodeApprover = z.infer<typeof ApprovalNodeApproverSchema>;

/**
* A TYPED decision-output declaration (#3447 P2 follow-up). The bare-string
* form of a `decisionOutputs` entry renders as free text; this form tells the
* decision UI which picker to render and whether to collect one id or many.
* The runtime treats `key` as the whitelist entry either way — `type` and
* `multiple` only shape the INPUT WIDGET, never the accepted value.
*/
export const DecisionOutputDefSchema = lazySchema(() => z.object({
/** The output key — what the flow receives as `<nodeId>.<key>`. */
key: z.string().min(1).describe('Output key (the flow variable name under the node id)'),
/** Display label for the decision-dialog field; defaults to a title-cased key. */
label: z.string().optional().describe('Field label in the decision dialog'),
/**
* Input widget: `text` (default — free text), or a record picker —
* `user` (sys_user), `department` (sys_business_unit), `position`
* (sys_position), `team` (sys_team). Picker values are record ids.
*/
type: z.enum(['text', 'user', 'department', 'position', 'team']).optional()
.describe("Decision-dialog input widget (default 'text')"),
/** Collect an id array instead of a single value (multi-select picker). */
multiple: z.boolean().optional().describe('Collect multiple values (id array)'),
}));
export type DecisionOutputDef = z.infer<typeof DecisionOutputDefSchema>;

/**
* Normalize a `decisionOutputs` array (bare keys and typed declarations mixed)
* into the typed form. The ONE place both sides read the union: the runtime
* whitelists `normalizeDecisionOutputs(...).map(d => d.key)`, the request read
* surfaces the normalized defs for the decision UI — so a bare key and
* `{ key }` can never behave differently.
*/
export function normalizeDecisionOutputs(
declared: unknown,
): DecisionOutputDef[] {
if (!Array.isArray(declared)) return [];
const out: DecisionOutputDef[] = [];
for (const entry of declared) {
if (typeof entry === 'string') {
if (entry) out.push({ key: entry });
} else if (entry && typeof entry === 'object' && typeof (entry as any).key === 'string' && (entry as any).key) {
const e = entry as Record<string, unknown>;
out.push({
key: String(e.key),
...(typeof e.label === 'string' && e.label ? { label: e.label } : {}),
...(typeof e.type === 'string' && e.type !== 'text' ? { type: e.type as DecisionOutputDef['type'] } : {}),
...(e.multiple === true ? { multiple: true } : {}),
});
}
}
return out;
}

/**
* Per-node SLA escalation — carried on the Approval node itself, so each
* Approval step on the canvas defines its own SLA.
Expand Down Expand Up @@ -291,9 +343,21 @@ export const ApprovalNodeConfigSchema = lazySchema(() => z.object({
* (`vars.<nodeId>.picked_departments`) — "the previous approver picks the
* next step's approvers" without a record-field detour. A decision carrying
* undeclared keys is rejected; `decision` / `requestId` are reserved.
*
* An entry is either a bare key (a plain text input in the decision UI) or a
* TYPED declaration — `{ key, type, multiple }` — that tells the decision UI
* to render a record picker instead of free text: `user` → a sys_user
* picker, `department`/`position`/`team` → the matching system-object
* picker. `multiple: true` collects an id ARRAY (the natural fan-out shape
* for an `expression` approver downstream). The type describes the INPUT
* WIDGET only — the runtime accepts whatever value shape arrives (single id,
* id[], CSV) and hands it to the flow verbatim.
*/
decisionOutputs: z.array(z.string()).optional()
.describe('Author-declared output keys a decision may carry (approvers fill values only)'),
decisionOutputs: z.array(z.union([
z.string(),
DecisionOutputDefSchema,
])).optional()
.describe('Author-declared decision outputs — bare keys or typed { key, type, multiple } declarations'),

/** Optional per-node SLA escalation. */
escalation: ApprovalEscalationSchema.optional().describe('Per-node SLA escalation'),
Expand Down
17 changes: 16 additions & 1 deletion packages/spec/src/contracts/approval-service.ts
Original file line number Diff line number Diff line change
Expand Up @@ -52,9 +52,24 @@ export interface ApprovalRequestRow {
* #3447 P2: the node's author-declared decision-output keys
* (`config.decisionOutputs`), surfaced from the config snapshot so a
* decision UI can render one input per key and POST `outputs` with the
* decision. Absent when the node declares none.
* decision. Absent when the node declares none. Kept as the bare KEY list
* for version skew — an older console renders these as text inputs.
*/
decision_outputs?: string[];
/**
* #3447 P2 follow-up: the normalized TYPED declarations behind
* `decision_outputs` — `{ key, label?, type?, multiple? }` — so a
* picker-aware decision UI renders a sys_user / department / position /
* team record picker (id values; `multiple` → id array) instead of free
* text. Always parallel to `decision_outputs`; consumers prefer this and
* fall back to the key list.
*/
decision_output_defs?: Array<{
key: string;
label?: string;
type?: 'text' | 'user' | 'department' | 'position' | 'team';
multiple?: boolean;
}>;
completed_at?: string;
created_at?: string;
updated_at?: string;
Expand Down
Loading