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
23 changes: 23 additions & 0 deletions .changeset/17931-approval-onemptyapprovers-fallback.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,23 @@
---
"@objectstack/spec": minor
"@objectstack/plugin-approvals": minor
"@objectstack/lint": minor
---

Approval nodes gain a fourth empty-slate policy — `onEmptyApprovers: 'fallback'` with a sibling `fallbackApprovers` list — so a rung that expands to nobody opens the request on people you named instead of on a slot nobody can act on.

Until now an approval node whose approvers resolved to nobody had three endings, and none of them named anyone: `admin_rescue` (the default — the request opens on a dead `type:value` slot and waits for a privileged admin), `fail` (the run dies) and `auto_approve` (the record is waved through). All five graph approver types reach that dead end, and `{ type: 'manager' }` reaches it without anybody authoring a wrong value: `manager` omits `value`, so the literal the expansion falls back to is `manager:undefined`.

```ts
{
approvers: [{ type: 'manager' }],
onEmptyApprovers: 'fallback',
fallbackApprovers: [{ type: 'org_membership_level', value: 'owner' }],
}
```

- **`fallbackApprovers` is the approver shape you already write** — the same entries as `approvers`, resolved by the same expansion, so every approver type, OOO delegation and `per_group` tagging behaves identically on it. It is not a second, reduced approver dialect.
- **The pairing is enforced in both directions.** `'fallback'` without a list is refused; a list under any other policy is refused too, because nothing would ever read it — a node that declares a rescue slate and silently ignores it is the failure this config shape is `.strict()` against. Both messages name both keys.
- **A fallback that itself resolves to nobody degrades to `admin_rescue`.** The run is never killed and the record is never waved through by a policy whose author only asked for different people; the log says both that the fallback fired and that it found nobody.
- **This is on the node, not on the `manager` rung** — the node is already where emptiness is decided, and a fallback is wanted for every approver type, not one of them.
- **`os lint` names the new escape and keeps firing without it.** `approval-approvers-may-resolve-empty` still reports a manager-only slate even when a fallback is declared: the rule reads shape, and a static check can no more prove a `fallbackApprovers` list resolves than it can read `sys_user.manager_id`. A seeded manager chain remains the one silencer.
41 changes: 35 additions & 6 deletions content/docs/automation/approvals.mdx
Original file line number Diff line number Diff line change
Expand Up @@ -94,10 +94,15 @@ write it either. Where an identity carries `source: 'idp_provisioned'` the admin
Setting the column is still something you have to *do*. Where `manager_id` is unset the rung
expands to nobody, the request opens on a slate no one can act on, and under the default
`lockRecord: true` the record stays locked — recoverable only by a platform or tenant admin
override. So set it for everyone who submits this request, or add a fallback approver that cannot
resolve empty — e.g. `{ type: 'org_membership_level', value: 'owner' }`. `os lint` reports the
shape at authoring time (`approval-approvers-may-resolve-empty`, `info`) and carries the same
remedy text, so the two cannot drift apart.
override. So set it for everyone who submits this request, or take one of the two escapes that
need no write to the column at all: add a second approver entry that cannot resolve empty — e.g.
`{ type: 'org_membership_level', value: 'owner' }` — or declare `onEmptyApprovers: 'fallback'`
on the node with a `fallbackApprovers` list, so an empty manager rung opens the request on those
people instead. Neither makes `manager` resolve; they make the failure survivable, which is a
different and independently useful thing. `os lint` reports the shape at authoring time
(`approval-approvers-may-resolve-empty`, `info`) and carries the same remedy text, so the two
cannot drift apart — and it keeps reporting it even with a fallback declared, because a static
check cannot read the column and cannot know the fallback resolves either.
</Callout>

Approving is itself a gated action — model "may approve" as a capability (`approve_invoice`) the approver's permission set grants, and gate the approve action's `requiredPermissions` on it so the gate is enforced on **both** the UI and the server (ADR-0066 D4).
Expand Down Expand Up @@ -273,12 +278,36 @@ empty slate you can see than a task the tenancy wall hides.
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
Reassign), `fail` (the node fails: an empty slate is a config bug),
`auto_approve` (skip the request and continue down `approve` with
`output.autoApproved = true`; opt-in, since it waves the record through). A
`output.autoApproved = true`; opt-in, since it waves the record through), or
`fallback` (open the request on the people named in `fallbackApprovers`). A
**missing** key (`vars.never_written`) is a loud error instead — guard
genuinely-optional inputs with `has(vars.x) ? vars.x : []`.

`fallback` is the only one of the four that **names people**, so it is the only
one that turns an empty slate into a request someone can actually decide:

```ts
{
approvers: [{ type: 'manager' }],
onEmptyApprovers: 'fallback',
fallbackApprovers: [{ type: 'org_membership_level', value: 'owner' }],
}
```

`fallbackApprovers` takes the same entries as `approvers` — same types, same
`group` / `resolveAs` / `organization` keys — and is resolved by the same
expansion, OOO delegation included. The pairing is enforced in both directions:
`fallback` without a `fallbackApprovers` list is refused at authoring time, and
a `fallbackApprovers` list under any other policy is refused too, because
nothing would ever read it.

Prefer a fallback target that cannot itself resolve empty (a literal `user`, or
the `owner` membership tier). If it does resolve empty the node degrades to
`admin_rescue` — the request still opens and the run is never killed — and the
log says both that the fallback fired and that it found nobody.

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
Expand Down
19 changes: 15 additions & 4 deletions content/docs/references/automation/approval.mdx
Original file line number Diff line number Diff line change
Expand Up @@ -52,7 +52,7 @@ const result = ApprovalDecision.parse(data);

| Property | Type | Required | Description |
| :--- | :--- | :--- | :--- |
| **type** | `Enum<'manager' \| 'position' \| 'department' \| 'team' \| 'field' \| 'expression' \| 'org_membership_level' \| 'role' \| 'user' \| 'queue'>` | ✅ | Approval step approver type. `manager` is a directory-sync dependency rather than something an author configures here: it resolves the submitter's `sys_user.manager_id` at runtime, and that column has no product write surface, so until an operator populates it from outside the product a manager step resolves to nobody and the request waits. `os lint` reports that at authoring time as `approval-approvers-may-resolve-empty` and carries the graded population routes and the full remedy; the Approvals guide states the same remedy in prose. |
| **type** | `Enum<'manager' \| 'position' \| 'department' \| 'team' \| 'field' \| 'expression' \| 'org_membership_level' \| 'role' \| 'user' \| 'queue'>` | ✅ | Approval step approver type. `manager` is resolved from the directory rather than configured here: it reads the submitter's `sys_user.manager_id` at runtime, and that column is not a profile field an author edits, so until an operator populates it through one of the routes `os lint` names, a manager step resolves to nobody and the request waits. `os lint` reports that at authoring time as `approval-approvers-may-resolve-empty` and carries the graded population routes and the full remedy, the node-level `onEmptyApprovers` fallback included; the Approvals guide states the same remedy in prose. |
| **value** | `string` | optional | User id / membership tier / position / team / department / field — 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 @@ -72,7 +72,8 @@ const result = ApprovalDecision.parse(data);
| **minApprovals** | `integer` | optional | Approvals required — total (quorum) or per group (per_group). Omitted ⇒ all resolvable approvers for quorum, 1 per group for per_group |
| **lockRecord** | `boolean` | optional (default: `true`) | 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'>` | optional (default: `"admin_rescue"`) | Behavior when no concrete approver resolves at node entry |
| **onEmptyApprovers** | `Enum<'admin_rescue' \| 'fail' \| 'auto_approve' \| 'fallback'>` | optional (default: `"admin_rescue"`) | Behavior when no concrete approver resolves at node entry — 'fallback' opens the request on fallbackApprovers instead |
| **fallbackApprovers** | `{ type: Enum<'manager' \| 'position' \| 'department' \| 'team' \| 'field' \| 'expression' \| …>; value?: string; resolveAs?: Enum<'user' \| 'department' \| 'position' \| 'team'>; group?: string; … }[]` | optional | Approvers the request opens on when onEmptyApprovers is 'fallback' |
| **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` | optional (default: `3`) | Max send-backs for revision before auto-reject (0 = send-back disabled) |
Expand All @@ -81,7 +82,17 @@ const result = ApprovalDecision.parse(data);

| Property | Type | Required | Description |
| :--- | :--- | :--- | :--- |
| **type** | `Enum<'manager' \| 'position' \| 'department' \| 'team' \| 'field' \| 'expression' \| …>` | ✅ | Approval step approver type. `manager` is a directory-sync dependency rather than something an author configures here: it resolves the submitter's `sys_user.manager_id` at runtime, and that column has no product write surface, so until an operator populates it from outside the product a manager step resolves to nobody and the request waits. `os lint` reports that at authoring time as `approval-approvers-may-resolve-empty` and carries the graded population routes and the full remedy; the Approvals guide states the same remedy in prose. |
| **type** | `Enum<'manager' \| 'position' \| 'department' \| 'team' \| 'field' \| 'expression' \| …>` | ✅ | Approval step approver type. `manager` is resolved from the directory rather than configured here: it reads the submitter's `sys_user.manager_id` at runtime, and that column is not a profile field an author edits, so until an operator populates it through one of the routes `os lint` names, a manager step resolves to nobody and the request waits. `os lint` reports that at authoring time as `approval-approvers-may-resolve-empty` and carries the graded population routes and the full remedy, the node-level `onEmptyApprovers` fallback included; the Approvals guide states the same remedy in prose. |
| **value** | `string` | optional | User id / membership tier / position / team / department / field — 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") |
| **organization** | `string` | optional | ADR-0105 D9 — organization whose directory resolves this approver: `$root` (group org), `$parent` (one level up), or an organization slug. Omitted = the request's own organization. |

### Nested Shape: `ApprovalNodeConfig.fallbackApprovers[number]`

| Property | Type | Required | Description |
| :--- | :--- | :--- | :--- |
| **type** | `Enum<'manager' \| 'position' \| 'department' \| 'team' \| 'field' \| 'expression' \| …>` | ✅ | Approval step approver type. `manager` is resolved from the directory rather than configured here: it reads the submitter's `sys_user.manager_id` at runtime, and that column is not a profile field an author edits, so until an operator populates it through one of the routes `os lint` names, a manager step resolves to nobody and the request waits. `os lint` reports that at authoring time as `approval-approvers-may-resolve-empty` and carries the graded population routes and the full remedy, the node-level `onEmptyApprovers` fallback included; the Approvals guide states the same remedy in prose. |
| **value** | `string` | optional | User id / membership tier / position / team / department / field — 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 Down Expand Up @@ -112,7 +123,7 @@ const result = ApprovalDecision.parse(data);

## ApproverType

Approval step approver type. `manager` is a directory-sync dependency rather than something an author configures here: it resolves the submitter's `sys_user.manager_id` at runtime, and that column has no product write surface, so until an operator populates it from outside the product a manager step resolves to nobody and the request waits. `os lint` reports that at authoring time as `approval-approvers-may-resolve-empty` and carries the graded population routes and the full remedy; the Approvals guide states the same remedy in prose.
Approval step approver type. `manager` is resolved from the directory rather than configured here: it reads the submitter's `sys_user.manager_id` at runtime, and that column is not a profile field an author edits, so until an operator populates it through one of the routes `os lint` names, a manager step resolves to nobody and the request waits. `os lint` reports that at authoring time as `approval-approvers-may-resolve-empty` and carries the graded population routes and the full remedy, the node-level `onEmptyApprovers` fallback included; the Approvals guide states the same remedy in prose.

### Allowed Values

Expand Down
52 changes: 52 additions & 0 deletions packages/lint/src/validate-approval-approvers.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -327,6 +327,37 @@ describe('unset-manager dead-end (#16748)', () => {
expect(finding.message).toContain('does not assert the slate IS empty');
});

it("names the node-level escape — onEmptyApprovers: 'fallback' with fallbackApprovers", () => {
const [finding] = validateApprovalApprovers(managerOnly());
expect(finding.hint).toContain("onEmptyApprovers: 'fallback'");
expect(finding.hint).toContain('fallbackApprovers');
// It is offered as a route that needs NO write to the column — the point
// of naming it beside an endpoint the operator may not be able to reach.
expect(finding.hint).toContain('needs no write to the column');
// ⛔ And the pre-existing routes are not displaced by it.
expect(finding.hint).toContain('/api/v1/auth/admin/set-user-manager');
expect(finding.hint).toContain("org_membership_level', value: 'owner'");
});

// ⛔ The load-bearing negative: declaring the policy must NOT silence the
// finding. This rule reads SHAPE, and a `fallbackApprovers` list can itself
// resolve to nobody at runtime — which a static check cannot see either. A
// silencer here would be a claim the rule is not allowed to make.
it('KEEPS FIRING when the node declares the fallback policy — the rule reads shape', () => {
const stack = managerOnly();
const cfg = (stack.flows as any)[0].nodes[1].config;
cfg.onEmptyApprovers = 'fallback';
cfg.fallbackApprovers = [{ type: 'org_membership_level', value: 'owner' }];

const findings = validateApprovalApprovers(stack);
expect(findings).toHaveLength(1);
expect(findings[0].rule).toBe(APPROVAL_APPROVERS_MAY_RESOLVE_EMPTY);
expect(findings[0].severity).toBe('info');

// The one silencer stays the one silencer.
expect(validateApprovalApprovers(withSeededManagerChain(stack))).toEqual([]);
});

// ── negative controls ──────────────────────────────────────────────────

it('NEGATIVE: a populated manager chain in the stack emits nothing', () => {
Expand Down Expand Up @@ -512,6 +543,27 @@ describe('expression approvers (#3447 P2)', () => {
expect(findings[0].hint).toContain('admin_rescue');
});

// The nudge enumerates the vocabulary, so it goes stale the moment the
// vocabulary widens. All four members, or an author picks from three.
it('enumerates every empty-slate policy in the nudge, fallback included', () => {
const [finding] = validateApprovalApprovers(stackWithConfig({
approvers: [{ type: 'expression', value: 'vars.picked' }],
}));
for (const member of ['admin_rescue', 'fail', 'auto_approve', 'fallback']) {
expect(finding.hint, `nudge omits ${member}`).toContain(member);
}
expect(finding.hint).toContain('fallbackApprovers');
});

it("accepts 'fallback' as an explicit empty policy — no nudge", () => {
const findings = validateApprovalApprovers(stackWithConfig({
approvers: [{ type: 'expression', value: 'vars.picked' }],
onEmptyApprovers: 'fallback',
fallbackApprovers: [{ type: 'user', value: 'u_backstop' }],
}));
expect(findings).toEqual([]);
});

it('errors on reserved decisionOutputs keys', () => {
const findings = validateApprovalApprovers(stackWithConfig({
approvers: [{ type: 'user', value: 'u1' }],
Expand Down
Loading
Loading