diff --git a/.changeset/typed-decision-outputs-3447.md b/.changeset/typed-decision-outputs-3447.md new file mode 100644 index 0000000000..ceefafcbc8 --- /dev/null +++ b/.changeset/typed-decision-outputs-3447.md @@ -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. diff --git a/content/docs/automation/approvals.mdx b/content/docs/automation/approvals.mdx index be644a178d..3a60f0b078 100644 --- a/content/docs/automation/approvals.mdx +++ b/content/docs/automation/approvals.mdx @@ -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 diff --git a/content/docs/references/automation/approval.mdx b/content/docs/references/automation/approval.mdx index 3382355cb1..62b2194a9c 100644 --- a/content/docs/references/automation/approval.mdx +++ b/content/docs/references/automation/approval.mdx @@ -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); @@ -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) | @@ -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) | + + +--- + diff --git a/examples/app-showcase/src/automation/flows/dynamic-approval.flow.ts b/examples/app-showcase/src/automation/flows/dynamic-approval.flow.ts index 8a15fee652..21419ddc53 100644 --- a/examples/app-showcase/src/automation/flows/dynamic-approval.flow.ts +++ b/examples/app-showcase/src/automation/flows/dynamic-approval.flow.ts @@ -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 }], }, }, { diff --git a/packages/lint/src/validate-approval-approvers.test.ts b/packages/lint/src/validate-approval-approvers.test.ts index 506cbefec7..edbc5b7a15 100644 --- a/packages/lint/src/validate-approval-approvers.test.ts +++ b/packages/lint/src/validate-approval-approvers.test.ts @@ -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' }], diff --git a/packages/lint/src/validate-approval-approvers.ts b/packages/lint/src/validate-approval-approvers.ts index 0dc5e969d1..3b310f40d5 100644 --- a/packages/lint/src/validate-approval-approvers.ts +++ b/packages/lint/src/validate-approval-approvers.ts @@ -42,6 +42,7 @@ import { APPROVAL_NODE_TYPE, DEPRECATED_APPROVER_TYPES, canonicalApproverType, + normalizeDecisionOutputs, } from '@objectstack/spec/automation'; import { collectCelRootIdentifiers } from '@objectstack/formula'; @@ -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({ diff --git a/packages/plugins/plugin-approvals/src/approval-service.test.ts b/packages/plugins/plugin-approvals/src/approval-service.test.ts index 3e48398e17..2ef565cfa7 100644 --- a/packages/plugins/plugin-approvals/src/approval-service.test.ts +++ b/packages/plugins/plugin-approvals/src/approval-service.test.ts @@ -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). diff --git a/packages/plugins/plugin-approvals/src/approval-service.ts b/packages/plugins/plugin-approvals/src/approval-service.ts index a409f4be82..6c4edabe53 100644 --- a/packages/plugins/plugin-approvals/src/approval-service.ts +++ b/packages/plugins/plugin-approvals/src/approval-service.ts @@ -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'; @@ -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; } @@ -1220,9 +1227,9 @@ export class ApprovalService implements IApprovalService { const outputKeys = input.outputs ? Object.keys(input.outputs) : []; let acceptedOutputs: Record | 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. ` diff --git a/packages/spec/api-surface.json b/packages/spec/api-surface.json index 78e8e1d617..4328ec5a00 100644 --- a/packages/spec/api-surface.json +++ b/packages/spec/api-surface.json @@ -2040,6 +2040,8 @@ "DataSourceConfigSchema (const)", "DataSyncConfig (type)", "DataSyncConfigSchema (const)", + "DecisionOutputDef (type)", + "DecisionOutputDefSchema (const)", "ETL (const)", "ETLDestination (type)", "ETLDestinationSchema (const)", @@ -2165,6 +2167,7 @@ "flowForm (const)", "getApprovalNodeConfigJsonSchema (function)", "importBpmnToConstructs (function)", + "normalizeDecisionOutputs (function)", "validateControlFlow (function)" ], "./api": [ diff --git a/packages/spec/json-schema.manifest.json b/packages/spec/json-schema.manifest.json index 14a2035397..fed14a6962 100644 --- a/packages/spec/json-schema.manifest.json +++ b/packages/spec/json-schema.manifest.json @@ -538,6 +538,7 @@ "automation/DataDestinationConfig", "automation/DataSourceConfig", "automation/DataSyncConfig", + "automation/DecisionOutputDef", "automation/ETLDestination", "automation/ETLEndpointType", "automation/ETLPipeline", diff --git a/packages/spec/src/automation/approval.zod.ts b/packages/spec/src/automation/approval.zod.ts index a22fc556be..7508e825b4 100644 --- a/packages/spec/src/automation/approval.zod.ts +++ b/packages/spec/src/automation/approval.zod.ts @@ -180,6 +180,58 @@ export const ApprovalNodeApproverSchema = lazySchema(() => z.object({ })); export type ApprovalNodeApprover = z.infer; +/** + * 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 `.`. */ + 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; + +/** + * 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; + 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. @@ -291,9 +343,21 @@ export const ApprovalNodeConfigSchema = lazySchema(() => z.object({ * (`vars..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'), diff --git a/packages/spec/src/contracts/approval-service.ts b/packages/spec/src/contracts/approval-service.ts index 58970171b7..698de94bda 100644 --- a/packages/spec/src/contracts/approval-service.ts +++ b/packages/spec/src/contracts/approval-service.ts @@ -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; diff --git a/skills/objectstack-automation/SKILL.md b/skills/objectstack-automation/SKILL.md index e9c3d32b42..4752ac1967 100644 --- a/skills/objectstack-automation/SKILL.md +++ b/skills/objectstack-automation/SKILL.md @@ -486,17 +486,54 @@ slate — guard genuinely-optional inputs explicitly, e.g. } ``` -The full "previous approver picks the next step's approvers" loop, end to end: +The full "previous approver picks the next step's approvers" loop, end to end +(the shipped `showcase_dynamic_approval` flow in the showcase app is this shape): + ```typescript -// Node A declares what a decision may hand to the flow: -{ id: 'lead_review', type: 'approval', - config: { approvers: [{ type: 'user', value: 'lead' }], decisionOutputs: ['next_reviewers'] } }, -// The lead approves with outputs: POST …/approve { outputs: { next_reviewers: ['u2','u3'] } } -// Node B resolves them at entry: -{ id: 'co_sign', type: 'approval', - config: { approvers: [{ type: 'expression', value: cel`vars.lead_review.next_reviewers` }], - behavior: 'unanimous', onEmptyApprovers: 'fail' } }, +import { defineFlow } from '@objectstack/spec'; + +export const DynamicApprovalFlow = defineFlow({ + name: 'dynamic_approval', + label: 'Dynamic Approval', + type: 'autolaunched', + status: 'active', + nodes: [ + { + id: 'start', type: 'start', label: 'On Submit', + config: { objectName: 'expense', triggerType: 'record-after-update', condition: "status == 'submitted'" }, + }, + { + // Node A declares what a decision may hand to the flow. The TYPED + // declaration renders a multi-select sys_user picker in the decision + // dialog; the lead approves with outputs: + // POST …/approve { outputs: { next_reviewers: ['u2', 'u3'] } } + id: 'lead_review', type: 'approval', label: 'Lead Review', + config: { + approvers: [{ type: 'org_membership_level', value: 'owner' }], + decisionOutputs: [{ key: 'next_reviewers', label: 'Next Reviewers', type: 'user', multiple: true }], + }, + }, + { + // Node B resolves them at entry from the lead's decision outputs. + id: 'co_sign', type: 'approval', label: 'Co-sign', + config: { + approvers: [{ type: 'expression', value: 'vars.lead_review.next_reviewers' }], + behavior: 'unanimous', + onEmptyApprovers: 'fail', + }, + }, + { id: 'approved', type: 'end', label: 'Approved' }, + { id: 'rejected', type: 'end', label: 'Rejected' }, + ], + edges: [ + { id: 'e1', source: 'start', target: 'lead_review' }, + { id: 'e2', source: 'lead_review', target: 'co_sign', label: 'approve' }, + { id: 'e3', source: 'lead_review', target: 'rejected', label: 'reject' }, + { id: 'e4', source: 'co_sign', target: 'approved', label: 'approve' }, + { id: 'e5', source: 'co_sign', target: 'rejected', label: 'reject' }, + ], +}); ``` Time-word cheat sheet across surfaces (do not mix them up): @@ -517,7 +554,7 @@ Time-word cheat sheet across surfaces (do not mix them up): | `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) | | `onEmptyApprovers` | #3447 — what an EMPTY resolved slate does: `admin_rescue` (default — request opens, only a privileged admin can act via Reassign; never waves through, never kills the run), `fail` (node fails — treat an empty slate as a config bug), `auto_approve` (skip the request, continue down `approve` with `output.autoApproved = true` — opt-in because it silently waves the record through). Declare it explicitly on any node with an `expression` approver (linted) | -| `decisionOutputs` | #3447 — keys a decision may carry as structured outputs (author declares keys, approvers fill values). Accepted outputs resume the run as `.` variables for downstream nodes; undeclared keys reject the decision; `decision`/`requestId` reserved | +| `decisionOutputs` | #3447 — decision outputs a decision may carry (author declares, approvers fill values). Entries are bare keys (free-text input) **or typed declarations** `{ key, label?, type: 'text'\|'user'\|'department'\|'position'\|'team', multiple? }` — a typed entry renders the matching record picker in the decision dialog (`multiple` collects an id array). Accepted outputs resume the run as `.` variables; undeclared keys reject the decision; `decision`/`requestId` reserved | | `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 |