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
18 changes: 18 additions & 0 deletions .changeset/expression-approvers-3447-p2.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,18 @@
---
"@objectstack/spec": minor
"@objectstack/formula": minor
"@objectstack/lint": minor
"@objectstack/service-automation": minor
"@objectstack/plugin-approvals": minor
"@objectstack/rest": minor
---

Dynamic approver routing for approval nodes (#3447 P2) — three new declarative capabilities:

**`expression` approvers.** A new approver type whose CEL expression resolves WHO approves at node entry, over exactly three roots: `current.*` (the record's live state), `trigger.*` (the submit-time snapshot) and `vars.*` (flow variables, incl. upstream node outputs). `record` and bare field names are rejected before evaluation — on this platform `record` always means "the record at event time", which is ambiguous at an approval node — with error messages that prescribe the correct spelling. The optional `resolveAs: 'user' | 'department' | 'position' | 'team'` re-expands each resolved id through the same graph lookups the static types use; with `behavior: 'per_group'` each intermediate value (e.g. each returned department) forms its own sign-off group. A missing key fails the node loudly; only a present-but-empty result counts as an empty slate.

**`onEmptyApprovers` policy.** What an empty resolved slate does, node-level, for all approver types: `admin_rescue` (default — request opens for privileged takeover, the #3424 behaviour), `fail` (node fails), or `auto_approve` (skip the request, continue down the `approve` edge with `output.autoApproved = true`). To support auto-approve, the automation engine now honours `NodeExecutionResult.branchLabel` on the synchronous completion path — the field existed but was only ever consumed via resume signals.

**Decision outputs.** `decide(..., { outputs })` hands structured data from the approver to the flow: the author declares allowed keys on the node (`decisionOutputs`), approvers fill values only, and accepted outputs resume the run as `<nodeId>.<key>` variables — a later approval node's expression can read `vars.<nodeId>.picked_departments`, closing "the previous approver picks the next step's approvers" without a record-field detour. Undeclared keys reject the decision; `decision`/`requestId` are reserved. Multi-approver tallies now always pin to the open-time approver snapshot (previously unanimous re-resolved at each decision against the payload snapshot).

Also: `collectCelRootIdentifiers` is exported from `@objectstack/formula` (shared by the new `os lint` rules and the runtime pre-check, so they can never drift), resolution inputs are audited on the request snapshot as `__resolvedFrom`, and three new lint rules gate expressions, empty-slate policies and reserved output keys at author time.
57 changes: 51 additions & 6 deletions content/docs/automation/approvals.mdx
Original file line number Diff line number Diff line change
Expand Up @@ -143,8 +143,50 @@ one node, no parallel branches needed:
A single **rejection** always finalizes the node as `rejected` (one veto), in
every mode. `minApprovals` is clamped to the resolvable approver count / group
size, so it can never deadlock. A decision may also carry `attachments` (file
references) recorded on its audit row — e.g. a signed contract. Weighted voting
and approval-matrix governance are enterprise, not here.
references) recorded on its audit row — e.g. a signed contract — and, when the
node declares `decisionOutputs`, structured `outputs` the flow receives as
`<nodeId>.<key>` variables (see [Dynamic approvers](#dynamic-approvers-3447)
below). Weighted voting and approval-matrix governance are enterprise, not here.

## Dynamic approvers (#3447) [#dynamic-approvers-3447]

Two approver kinds bind to **live** data at the moment the node is entered —
not the submit-time snapshot — so a step can route on data produced after
submit:

- **`field`** reads the record's live field value at node entry. An earlier
step (or an earlier step's approver) can write the routing field mid-flow and
the next approval node sees it; a multi-select user field fans out into one
approver slot per user.
- **`expression`** computes the slate with a CEL expression over exactly three
roots: **`current.*`** (the record's live state at node entry), **`trigger.*`**
(the submit-time snapshot — what a flow condition calls `record`), and
**`vars.*`** (flow variables, including node outputs and `vars.previous`).
`record` and bare field names are deliberately unavailable — at an approval
node "the record" is ambiguous between two times, so the expression must say
which one; referencing anything else fails the node loudly at entry rather
than resolving a silently-empty slate. The optional
`resolveAs: 'user' (default) | 'department' | 'position' | 'team'` re-expands
each resolved id through the same graph lookups the static types use — and
with `behavior: 'per_group'`, each intermediate value (e.g. each returned
department) forms its own sign-off group.

A result may legitimately be **empty** (a present-but-empty field or variable);
the node-level `onEmptyApprovers` policy decides what that means —
`admin_rescue` (default: the request opens, a privileged admin takes over via
Reassign), `fail` (the node fails: an empty slate is a config bug), or
`auto_approve` (skip the request and continue down `approve` with
`output.autoApproved = true`; opt-in, since it waves the record through). A
**missing** key (`vars.never_written`) is a loud error instead — guard
genuinely-optional inputs with `has(vars.x) ? vars.x : []`.

The "previous approver picks the next step's approvers" loop needs no record
field at all: declare `decisionOutputs: ['next_reviewers']` on node A, have the
approver decide with `{ outputs: { next_reviewers: ['u2', 'u3'] } }`, and let
node B's approver be
`{ type: 'expression', value: 'vars.<nodeA-id>.next_reviewers' }`. The author
declares output keys, approvers only fill values; undeclared keys reject the
decision, and `decision` / `requestId` are reserved.

## The full lifecycle — submit to field change

Expand All @@ -165,9 +207,12 @@ Only `approvers` is required on the node; everything else has a default
(`behavior: 'first_response'`, `lockRecord: true`, `maxRevisions: 3`).

Approver entries resolve by kind — `position`, `user`, `field`, `manager`,
`team`, `department`, `queue`, and `org_membership_level`, described in the
[callout above](#3-the-approval-node), which is the one that silently resolves
to nobody when it's mistaken for a business hierarchy. An entry that resolves
`team`, `department`, `queue`, `org_membership_level`, and `expression`
(described in the [callout above](#3-the-approval-node) and in
[Dynamic approvers](#dynamic-approvers-3447)); `org_membership_level` is the
one that silently resolves to nobody when it's mistaken for a business
hierarchy. `field`, `manager`, and `expression` resolve against the record's
**live** state at node entry (#3447). An entry that resolves
to nobody is not an error: the request opens with an empty `pending_approvers`
and nothing can move it, so the run parks forever.

Expand Down Expand Up @@ -246,7 +291,7 @@ continuity levers on the same request. All are `POST

| Verb | Route | Who | Effect |
|:---|:---|:---|:---|
| `approve` / `reject` | `/approve` `/reject` | pending approver | Records the decision (finalizes per `behavior`). Accepts `comment`, `attachments`. |
| `approve` / `reject` | `/approve` `/reject` | pending approver | Records the decision (finalizes per `behavior`). Accepts `comment`, `attachments`, and — when the node declares `decisionOutputs` — structured `outputs` handed to the flow as `<nodeId>.<key>` variables. |
| `reassign` | `/reassign` | pending approver | Hands one slot to another user (`to`); the request stays pending. |
| `revise` (send back) | `/revise` | pending approver | Ends this round as **`returned`**, unlocks the record for rework (ADR-0044). |
| `request-info` | `/request-info` | pending approver | Asks the submitter for more info; the request **stays pending**. `comment` required. |
Expand Down
10 changes: 7 additions & 3 deletions content/docs/references/automation/approval.mdx
Original file line number Diff line number Diff line change
Expand Up @@ -54,8 +54,9 @@ const result = ApprovalDecision.parse(data);

| Property | Type | Required | Description |
| :--- | :--- | :--- | :--- |
| **type** | `Enum<'user' \| 'org_membership_level' \| 'role' \| 'position' \| 'team' \| 'department' \| 'manager' \| 'field' \| 'queue'>` | ✅ | |
| **value** | `string` | optional | User id / membership tier / position / team / department / field / queue — per `type` |
| **type** | `Enum<'user' \| 'org_membership_level' \| 'role' \| 'position' \| 'team' \| 'department' \| 'manager' \| 'field' \| 'queue' \| 'expression'>` | ✅ | |
| **value** | `string` | optional | User id / membership tier / position / team / department / field / queue — per `type`; for `expression`, a CEL expression over `current.*` / `trigger.*` / `vars.*` |
| **resolveAs** | `Enum<'user' \| 'department' \| 'position' \| 'team'>` | optional | How an `expression` result is expanded into approvers (default 'user') |
| **group** | `string` | optional | Group label for per_group sign-off (e.g. "legal", "finance") |


Expand All @@ -67,11 +68,13 @@ const result = ApprovalDecision.parse(data);

| Property | Type | Required | Description |
| :--- | :--- | :--- | :--- |
| **approvers** | `{ type: Enum<'user' \| 'org_membership_level' \| 'role' \| 'position' \| 'team' \| 'department' \| 'manager' \| 'field' \| 'queue'>; value?: string; group?: string }[]` | ✅ | Allowed approvers for this node |
| **approvers** | `{ type: Enum<'user' \| 'org_membership_level' \| 'role' \| 'position' \| 'team' \| 'department' \| 'manager' \| 'field' \| 'queue' \| 'expression'>; value?: string; resolveAs?: Enum<'user' \| 'department' \| 'position' \| 'team'>; group?: string }[]` | ✅ | Allowed approvers for this node |
| **behavior** | `Enum<'first_response' \| 'unanimous' \| 'quorum' \| 'per_group'>` | ✅ | How to combine multiple approvers |
| **minApprovals** | `integer` | optional | Approvals required — total (quorum) or per group (per_group). Default 1 |
| **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) |
| **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 @@ -91,6 +94,7 @@ const result = ApprovalDecision.parse(data);
* `manager`
* `field`
* `queue`
* `expression`


---
Expand Down
47 changes: 46 additions & 1 deletion packages/formula/src/cel-engine.test.ts
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
import { describe, expect, it } from 'vitest';

import { celEngine, rewriteTemporalEquality } from './cel-engine';
import { celEngine, rewriteTemporalEquality, collectCelRootIdentifiers } from './cel-engine';
import { CEL_STDLIB_FUNCTIONS } from './validate';
import type { Expression } from '@objectstack/spec';

Expand Down Expand Up @@ -640,3 +640,48 @@ describe('celEngine', () => {
});
});
});

// ── collectCelRootIdentifiers (#3447 P2) ────────────────────────────────────
//
// Root extraction for closed-root evaluation sites (approval `expression`
// approvers). Lint and the runtime pre-check both consume this helper, so its
// contract IS the contract of what those sites accept.

describe('collectCelRootIdentifiers', () => {
const roots = (src: string) => {
const r = collectCelRootIdentifiers(src);
if (!r.ok) throw new Error(`expected parse ok: ${r.error}`);
return r.roots.sort();
};

it('collects distinct namespace roots from member chains', () => {
expect(roots('current.approvers_dynamic')).toEqual(['current']);
expect(roots('current.a.b.c == trigger.a && vars.step.result')).toEqual(['current', 'trigger', 'vars']);
});

it('reports a bare identifier as a root (the closed-site smoking gun)', () => {
expect(roots('approvers_dynamic')).toEqual(['approvers_dynamic']);
expect(roots('record.x')).toEqual(['record']);
});

it('never reports member names or function names', () => {
expect(roots('size(vars.picked) > 0 ? vars.picked : trigger.fallback')).toEqual(['trigger', 'vars']);
expect(roots('current.owner_id.startsWith("u")')).toEqual(['current']);
});

it('walks indexes, lists and nested calls', () => {
expect(roots('vars.picked[0] == current.ids[trigger.idx]')).toEqual(['current', 'trigger', 'vars']);
expect(roots('[current.a, vars.b]')).toEqual(['current', 'vars']);
});

it('accepts the #3306 null-guard ternary the same way compile does', () => {
expect(roots('current.n > 0 ? current.who : null')).toEqual(['current']);
});

it('reports a parse failure instead of an empty root set', () => {
const bad = collectCelRootIdentifiers('current..');
expect(bad.ok).toBe(false);
const empty = collectCelRootIdentifiers(' ');
expect(empty.ok).toBe(false);
});
});
50 changes: 50 additions & 0 deletions packages/formula/src/cel-engine.ts
Original file line number Diff line number Diff line change
Expand Up @@ -60,6 +60,11 @@ const SCOPE_ROOTS = [
// Master-detail inline grids inject the header record as `parent` for a
// child field's `readonlyWhen`/`requiredWhen` predicate (ADR-0036, #1581).
'parent',
// Approval-node `expression` approvers (#3447 P2): the record's LIVE state
// at node entry — bound only in that evaluation site, alongside `trigger`
// (the submit-time snapshot) and `vars`. Declared here so the strict lint
// env doesn't misread `current.x` as a bare field reference.
'current',
] as const;

/**
Expand Down Expand Up @@ -127,6 +132,51 @@ export function firstUndeclaredReference(
return null;
}

/**
* The distinct top-level identifiers (namespace roots) a CEL expression
* references — `current.x + vars.step.y` → `['current', 'vars']`, a bare
* `amount > 100` → `['amount']`. Member names and function names are not
* identifiers and are never reported.
*
* Built for evaluation sites that expose a CLOSED set of roots (#3447 P2:
* approval-node `expression` approvers allow only `current`/`trigger`/`vars`).
* Such a site must reject any other root BEFORE evaluating: the runtime env is
* `unlistedVariablesAreDyn: true`, so an out-of-contract root (`record.x`, a
* bare field) would otherwise evaluate to `null` and silently produce an empty
* result instead of an error. Both the lint rule and the runtime pre-check
* consume this one helper so the two can never drift.
*
* Returns `{ ok: false }` with the classifier's message when the source does
* not parse — callers surface that as a config error, not an empty root set.
*/
export function collectCelRootIdentifiers(
source: string,
): { ok: true; roots: string[] } | { ok: false; error: string } {
if (typeof source !== 'string' || !source.trim()) {
return { ok: false, error: 'expression is empty' };
}
try {
// Same nullable-ternary rewrite as compile/evaluate so "what parses" agrees
// across build, lint, and runtime (#3306).
const compiled = buildEnv(() => new Date(0)).parse(rewriteNullableTernary(source));
const roots = new Set<string>();
const walk = (node: unknown): void => {
if (Array.isArray(node)) { for (const child of node) walk(child); return; }
if (!isCelNode(node)) return; // member/function-name strings, literals
if (node.op === 'id' && typeof node.args === 'string') { roots.add(node.args); return; }
// Member access: only the receiver can hold identifiers — `args[1]` is the
// member NAME, which must not be reported as a root.
if (node.op === '.' && Array.isArray(node.args)) { walk(node.args[0]); return; }
walk(node.args);
};
walk(compiled.ast);
return { ok: true, roots: [...roots] };
} catch (err) {
const classified = classifyError(err);
return { ok: false, error: classified.ok === false ? classified.error.message : String(err) };
}
}

/**
* The result type cel-js's type-checker infers for a `value`/`predicate`
* expression — its raw CEL type name (`'int'`, `'double'`, `'string'`, `'bool'`,
Expand Down
4 changes: 4 additions & 0 deletions packages/formula/src/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,10 @@

export { ExpressionEngine, getEngine, hasDialect, register } from './registry';
export { celEngine, DEFAULT_LIMITS } from './cel-engine';
// #3447 P2 — root-identifier extraction for closed-root evaluation sites
// (approval `expression` approvers): lint and the runtime pre-check share this
// one helper so what they accept can never drift.
export { collectCelRootIdentifiers } from './cel-engine';
export { cronEngine } from './cron-engine';
export { templateEngine, TEMPLATE_FORMATTERS, formatValue } from './template-engine';
export { registerStdLib, buildScope } from './stdlib';
Expand Down
3 changes: 3 additions & 0 deletions packages/lint/src/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -127,6 +127,9 @@ export {
APPROVAL_APPROVER_TYPE_UNKNOWN,
APPROVAL_ESCALATION_REASSIGN_NO_TARGET,
APPROVAL_APPROVERS_MAY_RESOLVE_EMPTY,
APPROVAL_EXPRESSION_INVALID,
APPROVAL_EXPRESSION_NO_EMPTY_POLICY,
APPROVAL_DECISION_OUTPUTS_RESERVED,
} from './validate-approval-approvers.js';
export type { ApprovalApproverFinding, ApprovalApproverSeverity } from './validate-approval-approvers.js';

Expand Down
Loading