feat(approvals): decisionOutputs may be declared required (objectui#2955)

.changeset/required-decision-outputs.md

@@ -0,0 +1,14 @@
+---
+"@objectstack/spec": minor
+"@objectstack/plugin-approvals": minor
+---
+
+`decisionOutputs` entries may now be declared `required` (objectui#2955). A typed entry `{ key, label?, type?, multiple?, required?: true }` tells the runtime — not just the decision UI — that an approver must supply the value: an **approve** carrying no value, or a blank one (`''`, whitespace, `[]`, an array of blanks), is rejected with `VALIDATION_FAILED` before any write, so the audit row and the request are untouched and the run can never resume past the node with the key missing.
+
+That gap is what the flag closes. `decisionOutputs` exists so a decision can route the next step (`approvers: [{ type: 'expression', value: 'vars.lead_review.next_reviewers' }]`), but nothing made the approver actually answer: a skipped output resumed the run with the key absent, and the next node either faulted with `EXPRESSION_FAILED` or resolved an empty slate and stalled on `onEmptyApprovers: 'admin_rescue'` — long after the one person who could have filled it in had moved on. `onEmptyApprovers` was the only backstop, and it is a recovery mechanism, not a contract.
+
+**Reject never requires them.** The run leaves down the `reject` edge, where nothing reads the outputs — demanding routing data to say "no" would trap the rejection. Outputs still ride a reject when the approver filled them in.
+
+**No elevation bypass.** A one-click email action link and an `auto_approve` SLA escalation both fail the same way rather than advancing into a node that would resolve nobody; the escalation sweep already isolates a throwing request, so that decision stays pending and visibly overdue instead of silently breaking the run downstream. Enforcement is per decision, so on a `unanimous` / `quorum` node every approver supplies the required outputs and the finalizing decision's values are what the flow resumes with.
+
+`required` rides `normalizeDecisionOutputs`, so it reaches clients on `decision_output_defs` — a decision UI marks the field required and blocks locally instead of round-tripping to a 400. The console side ships in objectui#2955.

content/docs/automation/approvals.mdx

@@ -259,6 +259,19 @@ 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.
 
+Add **`required: true`** when the next step cannot run without it — the very
+case above, where node B has nobody to route to if the lead skipped the field.
+Unlike `type` / `multiple`, this one is enforced by the runtime: an **approve**
+carrying no value (or a blank one — `''`, `[]`) for a required key is rejected
+before any write, so the run can never resume past the node with the key
+missing. A **reject** never requires it: the run leaves down the `reject` edge,
+where nothing reads the outputs. Enforcement has no elevation bypass — a
+one-click email action link and an `auto_approve` SLA escalation both fail
+rather than advance into a node that would resolve nobody, leaving the request
+pending and visibly overdue. Without the flag your only backstop is
+`onEmptyApprovers` — the next node opens, resolves nobody, and stalls for an
+admin rescue long after the approver who could have filled it in has gone.
+
 ## The full lifecycle — submit to field change
 
 What actually happens between "the flow hits the approval node" and "the record

content/docs/references/automation/approval.mdx

@@ -81,7 +81,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 \| { 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 |
+| **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) |
 
@@ -116,6 +116,7 @@ const result = ApprovalDecision.parse(data);
 | **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) |
+| **required** | `boolean` | optional | Approver must supply this output to approve |
 
 
 ---

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

@@ -41,8 +41,12 @@ export const DynamicApprovalFlow = defineFlow({
  lockRecord: true,
  // 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 }],
+ // decision dialog (bare string keys render free text), and `required`
+ // is enforced on approve — stage 2 declares `onEmptyApprovers: 'fail'`,
+ // so a lead who skipped the field would kill the run.
+ decisionOutputs: [{
+ key: 'next_reviewers', label: 'Next Reviewers', type: 'user', multiple: true, required: true,
+ }],
  },
  },
  {

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

@@ -554,6 +554,115 @@ describe('ApprovalService (node era)', () => {
  expect(second.outputs).toEqual({ legal_note: 'ok', finance_note: 'ok too' });
  });
 
+ // ── required decision outputs (objectui#2955) ───────────────────
+ //
+ // `type`/`multiple` only shape the input widget; `required` is the one
+ // declaration the RUNTIME enforces. Without it the author's only backstop
+ // was `onEmptyApprovers`: the approve went through with the key missing, the
+ // next node's `expression` approver resolved nobody, and the run stalled for
+ // an admin rescue — long after the approver who could have supplied it left.
+
+ const requiredInput = (extra: Record<string, any> = {}) => openInput(['u9'], {}, {
+ decisionOutputs: [
+ { key: 'parallel_positions', label: 'Co-signing positions', type: 'position', multiple: true, required: true },
+ { key: 'note' },
+ ],
+ ...extra,
+ });
+
+ it('required outputs: surfaced on the request row so the UI can block before the round trip', async () => {
+ const req = await svc.openNodeRequest(requiredInput(), CTX) as any;
+ expect(req.decision_output_defs).toEqual([
+ { key: 'parallel_positions', label: 'Co-signing positions', type: 'position', multiple: true, required: true },
+ { key: 'note' },
+ ]);
+ });
+
+ it('required outputs: an approve that omits one is rejected before any write', async () => {
+ const req = await svc.openNodeRequest(requiredInput(), CTX);
+ await expect(svc.decideNode(req.id, {
+ decision: 'approve', actorId: 'u9', outputs: { note: 'looks fine' },
+ }, SYS)).rejects.toThrow(/VALIDATION_FAILED.*parallel_positions.*required to approve/s);
+ // Atomic: the decision left no audit row (only the open-time submit), and
+ // the request is untouched.
+ expect((engine._tables['sys_approval_action'] ?? []).map((a: any) => a.action)).toEqual(['submit']);
+ expect(engine._tables['sys_approval_request'][0].status).toBe('pending');
+ });
+
+ it('required outputs: an approve carrying no outputs at all is rejected too', async () => {
+ // The regression that matters — a decision surface that collects nothing
+ // (objectui#2955: the record header) used to sail straight through here.
+ const req = await svc.openNodeRequest(requiredInput(), CTX);
+ await expect(svc.decideNode(req.id, {
+ decision: 'approve', actorId: 'u9',
+ }, SYS)).rejects.toThrow(/VALIDATION_FAILED.*parallel_positions.*required to approve/s);
+ });
+
+ it('required outputs: present-but-blank does not satisfy the requirement', async () => {
+ // An untouched widget submits '' or [] — accepting those would hand the
+ // downstream expression approver an empty slate, i.e. the exact stall.
+ for (const blank of ['', ' ', [], [''], null]) {
+ engine._tables['sys_approval_request'] = [];
+ engine._tables['sys_approval_action'] = [];
+ const req = await svc.openNodeRequest(requiredInput(), CTX);
+ await expect(svc.decideNode(req.id, {
+ decision: 'approve', actorId: 'u9', outputs: { parallel_positions: blank },
+ }, SYS)).rejects.toThrow(/VALIDATION_FAILED.*required to approve/s);
+ }
+ });
+
+ it('required outputs: a filled approve goes through and hands the value to the flow', async () => {
+ const req = await svc.openNodeRequest(requiredInput(), CTX);
+ const out = await svc.decideNode(req.id, {
+ decision: 'approve', actorId: 'u9', outputs: { parallel_positions: ['pos_1', 'pos_2'] },
+ }, SYS);
+ expect(out.finalized).toBe(true);
+ expect(out.outputs).toEqual({ parallel_positions: ['pos_1', 'pos_2'] });
+ });
+
+ it('required outputs: a REJECT never needs them', async () => {
+ // The run leaves down the reject edge, where nothing reads the outputs —
+ // demanding routing data to say "no" would trap the rejection.
+ const req = await svc.openNodeRequest(requiredInput(), CTX);
+ const out = await svc.decideNode(req.id, { decision: 'reject', actorId: 'u9' }, SYS);
+ expect(out.finalized).toBe(true);
+ expect(out.decision).toBe('reject');
+ });
+
+ it('required outputs: an unrelated node is unaffected', async () => {
+ // No `required` anywhere → the pre-#2955 behaviour, byte for byte.
+ const req = await svc.openNodeRequest(
+ openInput(['u9'], {}, { decisionOutputs: ['note'] }), CTX,
+ );
+ const out = await svc.decideNode(req.id, { decision: 'approve', actorId: 'u9' }, SYS);
+ expect(out.finalized).toBe(true);
+ });
+
+ it('required outputs: every approver of a multi-approver node must supply them', async () => {
+ // Enforced per decision, not per node: each approve is a decision that
+ // could be the one that resumes the run, and each approver sees the field.
+ const req = await svc.openNodeRequest(
+ openInput(['u1', 'u2'], {}, {
+ behavior: 'unanimous',
+ decisionOutputs: [{ key: 'co_signer', type: 'user', required: true }],
+ }), CTX,
+ );
+ await expect(svc.decideNode(req.id, {
+ decision: 'approve', actorId: 'u1',
+ }, SYS)).rejects.toThrow(/VALIDATION_FAILED.*co_signer.*required to approve/s);
+ const first = await svc.decideNode(req.id, {
+ decision: 'approve', actorId: 'u1', outputs: { co_signer: 'u7' },
+ }, SYS);
+ expect(first.finalized).toBe(false);
+ const second = await svc.decideNode(req.id, {
+ decision: 'approve', actorId: 'u2', outputs: { co_signer: 'u8' },
+ }, SYS);
+ expect(second.finalized).toBe(true);
+ // Last decision wins the merge — the finalizing approver's pick is what
+ // the flow resumes with.
+ expect(second.outputs).toEqual({ co_signer: 'u8' });
+ });
+
  // ── approver expansion: position (ADR-0090 D3) ──────────────────
 
  const positionInput = (extra: Record<string, any> = {}) => ({

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

@@ -278,6 +278,23 @@ function csvSplit(raw: unknown): string[] {
  return String(raw).split(',').map(s => s.trim()).filter(Boolean);
 }
 
+/**
+ * Is a submitted decision output blank — i.e. does it fail a `required`
+ * declaration (objectui#2955)?
+ *
+ * "Present but empty" has to count as missing: a decision UI sends whatever
+ * its widget holds, and an untouched picker/text box is `''` or `[]`. Letting
+ * those through would satisfy `required` with a value the downstream
+ * `expression` approver then resolves to nobody — the exact stall the flag
+ * exists to prevent. `false` and `0` are real values and pass.
+ */
+function isBlankDecisionOutput(value: unknown): boolean {
+ if (value === undefined || value === null) return true;
+ if (typeof value === 'string') return value.trim() === '';
+ if (Array.isArray(value)) return value.filter(v => v !== null && v !== undefined && String(v).trim() !== '').length === 0;
+ return false;
+}
+
 /**
  * Humanize a machine name for display fallback: strips a `flow:` prefix and
  * title-cases underscore/dash segments (`flow:manager_review` → "Manager
@@ -1641,10 +1658,11 @@ export class ApprovalService implements IApprovalService {
  // caller bug; `decision`/`requestId` are reserved by the resume envelope.
  const outputKeys = input.outputs ? Object.keys(input.outputs) : [];
  let acceptedOutputs: Record<string, unknown> | undefined;
+ // Typed declarations and bare keys whitelist identically — one normalizer
+ // (spec) is the single reader of the union shape.
+ const declaredDefs = normalizeDecisionOutputs((config as any).decisionOutputs);
  if (outputKeys.length) {
- // 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);
+ const declared = declaredDefs.map(d => d.key);
  if (!declared.length) {
  throw new Error(
  `VALIDATION_FAILED: this approval node declares no decisionOutputs — outputs are not accepted. `
@@ -1669,6 +1687,35 @@ export class ApprovalService implements IApprovalService {
  acceptedOutputs = { ...input.outputs };
  }
 
+ // objectui#2955: `required` outputs. Unlike `type`/`multiple` — which only
+ // shape the input widget — this one is a runtime contract: the flow must
+ // never resume past this node with a required key missing, because that is
+ // precisely what a downstream `expression` approver reads. Before this,
+ // an author's only backstop was `onEmptyApprovers` (the next node opens,
+ // resolves nobody, and stalls for an admin rescue).
+ //
+ // APPROVE only. A reject leaves down the reject edge, where the outputs
+ // are not read — demanding routing data to say "no" would block the
+ // rejection. Outputs still ride a reject when the approver filled them.
+ //
+ // Enforced for EVERY approve path, with no elevation bypass: a one-click
+ // email action link cannot fill a form, and an `auto_approve` SLA
+ // escalation has nobody to ask — both must fail rather than resume the run
+ // with the key missing. The escalation sweep already isolates a throwing
+ // request (it catches per-request), so that decision simply stays pending
+ // and visibly overdue instead of advancing into a broken node.
+ if (input.decision === 'approve') {
+ const missing = declaredDefs
+ .filter(d => d.required === true && isBlankDecisionOutput(input.outputs?.[d.key]))
+ .map(d => d.key);
+ if (missing.length) {
+ throw new Error(
+ `VALIDATION_FAILED: decision output(s) \`${missing.join('`, `')}\` are required to approve this `
+ + `request — open the approval and fill them in before approving.`,
+ );
+ }
+ }
+
  // Audit the decision first so the quorum/per_group tally below sees it.
  await this.engine.insert('sys_approval_action', {
  id: uid('aact'), request_id: requestId, organization_id: org,

packages/spec/authorable-surface.json

@@ -2287,6 +2287,7 @@
  "automation/DecisionOutputDef:key",
  "automation/DecisionOutputDef:label",
  "automation/DecisionOutputDef:multiple",
+ "automation/DecisionOutputDef:required",
  "automation/DecisionOutputDef:type",
  "automation/ETLDestination:config",
  "automation/ETLDestination:connector",

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

@@ -14,6 +14,7 @@ import {
  ApprovalEscalationSchema,
  ApprovalNodeConfigSchema,
  getApprovalNodeConfigJsonSchema,
+ normalizeDecisionOutputs,
 } from './approval.zod';
 
 describe('ApproverType', () => {
@@ -280,3 +281,39 @@ describe('ApprovalEscalationSchema', () => {
  expect(() => ApprovalEscalationSchema.parse({ enabled: true, timeoutHours: 0 })).toThrow();
  });
 });
+
+/**
+ * `normalizeDecisionOutputs` is the ONE reader of the bare-key | typed-object
+ * union — the runtime whitelists from it, and the request row surfaces its
+ * output to the decision UI. Anything it drops is invisible to both sides,
+ * which is how `required` has to be pinned here and not only at the service.
+ */
+describe('normalizeDecisionOutputs', () => {
+ it('lifts bare keys into the typed form', () => {
+ expect(normalizeDecisionOutputs(['next_reviewers', 'note'])).toEqual([
+ { key: 'next_reviewers' },
+ { key: 'note' },
+ ]);
+ });
+
+ it('carries the widget hints and the required flag through (objectui#2955)', () => {
+ expect(normalizeDecisionOutputs([
+ { key: 'positions', label: 'Co-signers', type: 'position', multiple: true, required: true },
+ ])).toEqual([
+ { key: 'positions', label: 'Co-signers', type: 'position', multiple: true, required: true },
+ ]);
+ });
+
+ it('omits the falsy flags rather than spelling them out', () => {
+ // The normalized shape is what the request row ships to every client, so
+ // an absent flag must stay absent instead of becoming `required: false`.
+ expect(normalizeDecisionOutputs([{ key: 'note', multiple: false, required: false }]))
+ .toEqual([{ key: 'note' }]);
+ });
+
+ it('drops entries with no usable key, and non-arrays entirely', () => {
+ expect(normalizeDecisionOutputs(['', { key: '' }, { label: 'no key' }, 42])).toEqual([]);
+ expect(normalizeDecisionOutputs(undefined)).toEqual([]);
+ expect(normalizeDecisionOutputs('next_reviewers')).toEqual([]);
+ });
+});

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

@@ -457,6 +457,22 @@ export const DecisionOutputDefSchema = lazySchema(() => z.object({
  .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)'),
+ /**
+ * The approver must supply this output to APPROVE (objectui#2955).
+ *
+ * Unlike `type`/`multiple` — which only shape the input widget — this one
+ * IS enforced by the runtime: an approve carrying no value for a required
+ * key is rejected before any write, so a downstream `expression` approver
+ * reading `vars.<nodeId>.<key>` can never face a missing key. Without it an
+ * author's only backstop was `onEmptyApprovers` — the run reaches the next
+ * node, finds nobody, and stalls for an admin rescue.
+ *
+ * NOT enforced on reject: the run leaves down the reject edge, where the
+ * outputs are not read, and demanding routing data to say "no" would block
+ * the rejection. Outputs are still accepted on a reject if the approver
+ * filled them in.
+ */
+ required: z.boolean().optional().describe('Approver must supply this output to approve'),
 }));
 export type DecisionOutputDef = z.infer<typeof DecisionOutputDefSchema>;
 
@@ -482,6 +498,7 @@ export function normalizeDecisionOutputs(
  ...(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 } : {}),
+ ...(e.required === true ? { required: true } : {}),
  });
  }
  }