Skip to content

Commit e10d4e2

Browse files
committed
feat(approvals): cross-organization approver targeting (ADR-0105 D9)
One organization id decided three different things at once in `openNodeRequest`: where the request row lives, where its inbox index rows live, and where its approvers are looked up. The first two are the request's own organization by definition. The third is not — a group CFO holds her `cfo` position in the GROUP organization while the purchase order she signs off lives in the PLANT organization. `expandPositionUsers('cfo', <plant>)` matched nobody, the slot fell back to the dead `position:cfo` literal, and a group escalation could not be expressed at all. An approver may now declare which organization's directory resolves it: - { type: position, value: plant_manager, group: plant } - { type: position, value: cfo, organization: $root, group: finance } Four design points the ADR left open, settled from what the code and the authoring model require (ADR text amended in this commit): - PER APPROVER, not per node. The node-level form cannot express the commonest group shape — one node requiring a plant manager AND a group CFO in parallel. Splitting into serial nodes changes the semantics rather than the syntax. A node-level default remains addable later as sugar; the reverse is not true. - SYMBOLS FIRST (`$root` / `$parent`). Flow metadata is portable across environments; an organization id is data minted per deployment, so a literal id in a flow is unportable by construction and an AI author cannot know one. The symbols walk D6's `parent_organization_id` tree with zero deployment knowledge. A slug covers what they cannot — notably a SIBLING organization (a shared-services centre approving payables for every plant). - LEGALITY IS "SHARES A ROOT", not "is an ancestor" — the sibling case above is first-class. The rule reads only the organization tree, never the submitter, so one flow routes identically for everyone and a routing bug is reproducible. - NON-`group` POSTURES REFUSE at runtime. Posture is environment configuration, so the same portable metadata deploys into any posture and no static check can see which; ignoring the declaration would let a group → isolated migration reroute approvals with no signal. Two failure modes are made loud rather than silent: an approver type with no org-scoped directory (`user`/`field`/`manager`/`team`) refuses the declaration instead of ignoring it, with a new `approval-approver-cross-org-unsupported` lint catching it at author time; and a targeted approver holding no membership in the request's organization is dropped with a warning naming them — D2's union wall would otherwise hide the request from someone already routed to, leaving a task she cannot open. Dropping hands it to the node's existing `onEmptyApprovers` policy. An approver without `organization` is unchanged: same resolution, same queries, no extra reads. Tests: 17 resolver units (symbols, sibling/foreign slugs, posture, cycles, unreadable-membership), 7 service integration (including the pre-D9 gap, the mixed plant+group node, and that the request still belongs to the request org), 4 lint. plugin-approvals 305, lint 500, spec 6736, cli 682 all green. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_015FebXPaaGrLhGKw1LHPbpL
1 parent 3ea7271 commit e10d4e2

13 files changed

Lines changed: 1001 additions & 13 deletions
Lines changed: 53 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,53 @@
1+
---
2+
"@objectstack/plugin-approvals": minor
3+
"@objectstack/spec": minor
4+
"@objectstack/lint": minor
5+
---
6+
7+
feat(approvals): cross-organization approver targeting — a plant document can
8+
require a group-side sign-off (ADR-0105 D9)
9+
10+
One organization id used to decide three different things at once in
11+
`openNodeRequest`: where the request row lives, where its inbox index rows
12+
live, and **where its approvers are looked up**. The first two are the
13+
request's own organization by definition. The third is not — a group CFO holds
14+
her `cfo` position in the GROUP organization while the purchase order she signs
15+
off lives in the PLANT organization. `expandPositionUsers('cfo', <plant>)`
16+
matched nobody, the slot fell back to the dead `position:cfo` literal, and a
17+
group escalation could not be expressed at all.
18+
19+
An approver may now declare which organization's directory resolves it:
20+
21+
```yaml
22+
approvers:
23+
- { type: position, value: plant_manager, group: plant }
24+
- { type: position, value: cfo, organization: $root, group: finance }
25+
behavior: per_group
26+
```
27+
28+
- **`$root` / `$parent`** walk D6's `parent_organization_id` tree, so the two
29+
common intents need **no deployment knowledge** — flow metadata is portable
30+
across environments while organization ids are minted per deployment. A slug
31+
covers what the symbols cannot, notably a **sibling** organization (a
32+
shared-services centre approving payables for every plant).
33+
- Declared **per approver**, so one node can require a plant manager and a
34+
group CFO in parallel. A node-level form cannot express that without
35+
splitting into serial nodes, which changes the semantics.
36+
- **Bounded, not free:** the target must share a `parent_organization_id` root
37+
with the request's organization. The rule reads only the organization tree —
38+
never the submitter — so one flow routes identically for everyone.
39+
40+
Everything else fails loudly rather than quietly:
41+
42+
- a non-`group` posture **refuses** the declaration (a `group` → `isolated`
43+
migration must not silently reroute approvals);
44+
- an approver type with no org-scoped directory (`user` / `field` / `manager` /
45+
`team`) refuses it too, and a new `approval-approver-cross-org-unsupported`
46+
lint catches that at author time;
47+
- a targeted approver holding no membership in the request's organization is
48+
dropped with a warning naming them — D2's union wall would otherwise hide the
49+
request from someone already routed to, so the node's existing
50+
`onEmptyApprovers` policy takes over instead of leaving an unopenable task.
51+
52+
Nothing changes for an approver without `organization`: same resolution, same
53+
queries, no extra reads.

content/docs/references/automation/approval.mdx

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -58,6 +58,7 @@ const result = ApprovalDecision.parse(data);
5858
| **value** | `string` | optional | User id / membership tier / position / team / department / field — per `type`; for `expression`, a CEL expression over `current.*` / `trigger.*` / `vars.*` |
5959
| **resolveAs** | `Enum<'user' \| 'department' \| 'position' \| 'team'>` | optional | How an `expression` result is expanded into approvers (default 'user') |
6060
| **group** | `string` | optional | Group label for per_group sign-off (e.g. "legal", "finance") |
61+
| **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. |
6162

6263

6364
---
@@ -68,7 +69,7 @@ const result = ApprovalDecision.parse(data);
6869

6970
| Property | Type | Required | Description |
7071
| :--- | :--- | :--- | :--- |
71-
| **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 |
72+
| **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 |
7273
| **behavior** | `Enum<'first_response' \| 'unanimous' \| 'quorum' \| 'per_group'>` || How to combine multiple approvers |
7374
| **minApprovals** | `integer` | optional | Approvals required — total (quorum) or per group (per_group). Default 1 |
7475
| **lockRecord** | `boolean` || Lock the record from editing while pending |

docs/adr/0105-group-tenancy-posture-and-first-class-org-scope.md

Lines changed: 45 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -249,14 +249,53 @@ the `single`-posture deployment (plant-autonomous member admission) and is the
249249
natural admission UX in `group` posture.
250250

251251
**D9 — Cross-org approval targeting.**
252-
An approval chain node may name a **target organization** for approver
253-
resolution (default: the request's org, today's behavior at
254-
`approval-node.ts:118`). A plant document's escalation step declares
255-
`organization: <group org>` and resolves group-side position holders. Reads of
256-
the request by those approvers are covered by D2 (membership union) in `group`
257-
posture; in `isolated` posture this decision does not apply (cross-org
252+
An approval **approver** may name a **target organization** for its resolution
253+
(default: the request's org, today's behavior). A plant document's escalation
254+
step declares `organization: $root` and resolves group-side position holders.
255+
Reads of the request by those approvers are covered by D2 (membership union) in
256+
`group` posture; in `isolated` posture this decision does not apply (cross-org
258257
approval there remains mirroring via system context, cloud #2937 contract).
259258

259+
*Amended during implementation (#3812). Four points the original text left
260+
open, each settled from what the code and the authoring model actually
261+
require:*
262+
263+
1. *Per **approver**, not per node.* The node-level form cannot express the
264+
commonest group shape — one node requiring a plant manager **and** a group
265+
CFO in parallel (`behavior: 'per_group'`). Expressing it as two serial nodes
266+
changes the semantics (parallel co-sign becomes sequential approval), so the
267+
node-level form distorts the model rather than merely inconveniencing the
268+
author. A node-level default is a strict special case of the per-approver
269+
form and remains addable later as sugar; the reverse is not true.
270+
2. *Symbolic references first: `$root`, `$parent`.* Flow metadata is portable
271+
across environments; an organization id is data minted per deployment. A
272+
literal id in a flow is therefore unportable by construction, and an AI
273+
author cannot know one at authoring time. The symbols express the two common
274+
intents against D6's tree with **zero deployment knowledge**. A slug covers
275+
what they cannot — notably a **sibling** organization (a shared-services
276+
centre approving payables for every plant).
277+
3. *Legality is "shares a `parent_organization_id` root", not "is an
278+
ancestor".* The sibling case above is first-class, as is downward targeting.
279+
The rule depends only on the organization tree and **never on the
280+
submitter**, so one flow routes identically for everyone — which is what
281+
makes a routing bug reproducible. A deployment with no grouping metadata
282+
gets a refusal naming D6, not a silent pass.
283+
4. *Non-`group` postures **refuse** at runtime rather than ignoring.* Posture is
284+
environment configuration, so the same portable metadata may be deployed
285+
into any posture and no static check can see which. Silently ignoring the
286+
declaration would let a `group``isolated` migration reroute approvals
287+
with no signal — an audit event, not a config detail.
288+
289+
*Two consequences worth stating, both enforced:* an approver type that resolves
290+
no org-scoped directory (`user` / `field` / `manager` / `team`) **refuses** the
291+
declaration instead of ignoring it (an author who wrote it believed it did
292+
something); and a targeted approver who holds no membership in the request's
293+
organization is **dropped with a warning** naming them, because D2's union
294+
would otherwise hide the request from someone the router had already committed
295+
to — routing succeeds, the inbox row is written, and the approver opens a task
296+
she cannot open. Dropping converts that silent dead-end into the node's
297+
existing `onEmptyApprovers` policy.
298+
260299
**D10 — Layered master data (group template + org override).**
261300
A spec-level pattern for the SAP material-master / 用友-金蝶 distribution
262301
shape: an object may declare layered governance — group-level template rows

packages/lint/src/index.ts

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -135,6 +135,7 @@ export {
135135
APPROVAL_EXPRESSION_INVALID,
136136
APPROVAL_EXPRESSION_NO_EMPTY_POLICY,
137137
APPROVAL_DECISION_OUTPUTS_RESERVED,
138+
APPROVAL_APPROVER_CROSS_ORG_UNSUPPORTED,
138139
} from './validate-approval-approvers.js';
139140
export type { ApprovalApproverFinding, ApprovalApproverSeverity } from './validate-approval-approvers.js';
140141

packages/lint/src/validate-approval-approvers.test.ts

Lines changed: 37 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -12,6 +12,7 @@ import {
1212
APPROVAL_EXPRESSION_INVALID,
1313
APPROVAL_EXPRESSION_NO_EMPTY_POLICY,
1414
APPROVAL_DECISION_OUTPUTS_RESERVED,
15+
APPROVAL_APPROVER_CROSS_ORG_UNSUPPORTED,
1516
} from './validate-approval-approvers.js';
1617

1718
function stackWithApprovers(approvers: unknown[]): Record<string, unknown> {
@@ -334,3 +335,39 @@ describe('expression approvers (#3447 P2)', () => {
334335
}))).toEqual([]);
335336
});
336337
});
338+
339+
describe('cross-organization targeting (ADR-0105 D9)', () => {
340+
it('is clean when `organization` sits on an org-scoped approver type', () => {
341+
// The whole point of D9: a group CFO resolved against the group directory.
342+
const findings = validateApprovalApprovers(
343+
stackWithApprovers([{ type: 'position', value: 'cfo', organization: '$root' }]),
344+
);
345+
expect(findings.filter(f => f.rule === APPROVAL_APPROVER_CROSS_ORG_UNSUPPORTED)).toEqual([]);
346+
});
347+
348+
it('errors when `organization` sits on a type with no organization directory', () => {
349+
// `user` names a person outright — the declaration cannot narrow anything,
350+
// and the runtime refuses it. Catch it at author time instead.
351+
const findings = validateApprovalApprovers(
352+
stackWithApprovers([{ type: 'user', value: 'u1', organization: '$root' }]),
353+
);
354+
const f = findings.find(x => x.rule === APPROVAL_APPROVER_CROSS_ORG_UNSUPPORTED);
355+
expect(f?.severity).toBe('error');
356+
expect(f?.path).toBe('flows[0].nodes[1].config.approvers[0].organization');
357+
expect(f?.hint).toMatch(/position.*org_membership_level.*department.*expression/);
358+
});
359+
360+
it('errors for `team` too — team membership carries no organization', () => {
361+
const findings = validateApprovalApprovers(
362+
stackWithApprovers([{ type: 'team', value: 't1', organization: 'acme-ssc' }]),
363+
);
364+
expect(findings.some(f => f.rule === APPROVAL_APPROVER_CROSS_ORG_UNSUPPORTED)).toBe(true);
365+
});
366+
367+
it('stays silent when no organization is declared — the default path', () => {
368+
const findings = validateApprovalApprovers(
369+
stackWithApprovers([{ type: 'user', value: 'u1' }]),
370+
);
371+
expect(findings.filter(f => f.rule === APPROVAL_APPROVER_CROSS_ORG_UNSUPPORTED)).toEqual([]);
372+
});
373+
});

packages/lint/src/validate-approval-approvers.ts

Lines changed: 28 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -24,6 +24,7 @@
2424
* | approval-expression-invalid | error/info | #3447 P2 closed-root expressions |
2525
* | approval-expression-no-empty-policy | info | #3447 P2 empty-slate policy |
2626
* | approval-decision-outputs-reserved | error | #3447 P2 resume envelope |
27+
* | approval-approver-cross-org-unsupported | error | ADR-0105 D9 targeting |
2728
*
2829
* The first two are mutually exclusive by construction — a bad *value* wins,
2930
* because its fix (`position`) differs from the deprecation's fix
@@ -42,6 +43,7 @@ import {
4243
APPROVAL_NODE_TYPE,
4344
DEPRECATED_APPROVER_TYPES,
4445
APPROVER_VALUE_BINDINGS,
46+
approverTypeIsOrgScoped,
4547
canonicalApproverType,
4648
normalizeDecisionOutputs,
4749
} from '@objectstack/spec/automation';
@@ -57,6 +59,7 @@ export const APPROVAL_APPROVERS_MAY_RESOLVE_EMPTY = 'approval-approvers-may-reso
5759
export const APPROVAL_EXPRESSION_INVALID = 'approval-expression-invalid';
5860
export const APPROVAL_EXPRESSION_NO_EMPTY_POLICY = 'approval-expression-no-empty-policy';
5961
export const APPROVAL_DECISION_OUTPUTS_RESERVED = 'approval-decision-outputs-reserved';
62+
export const APPROVAL_APPROVER_CROSS_ORG_UNSUPPORTED = 'approval-approver-cross-org-unsupported';
6063

6164
/**
6265
* The CLOSED root set an `expression` approver may reference (#3447 P2) —
@@ -304,6 +307,31 @@ export function validateApprovalApprovers(stack: AnyRec): ApprovalApproverFindin
304307
`Queue approvers need a real ownership-queue implementation before they take effect.`,
305308
});
306309
}
310+
311+
// [ADR-0105 D9] Cross-organization targeting on a type that has no
312+
// organization-scoped directory. `user` / `field` / `manager` name a
313+
// person outright and `team` membership carries no organization, so the
314+
// declaration cannot narrow anything — it is a misunderstanding of what
315+
// the field does, and the runtime refuses it. Error, not warning: this
316+
// is a certain authoring mistake with a certain fix, and letting it
317+
// reach the runtime turns author time into an incident.
318+
const declaredOrg = (a as AnyRec).organization;
319+
if (typeof declaredOrg === 'string' && declaredOrg.trim() !== ''
320+
&& ApproverType.options.includes(canonical as never)
321+
&& !approverTypeIsOrgScoped(canonical)) {
322+
findings.push({
323+
severity: 'error',
324+
rule: APPROVAL_APPROVER_CROSS_ORG_UNSUPPORTED,
325+
where,
326+
path: `${path}.organization`,
327+
message:
328+
`approver type '${type}' does not resolve through an organization directory, so ` +
329+
`'organization: ${declaredOrg}' has no effect (ADR-0105 D9) — the runtime refuses it.`,
330+
hint:
331+
`Drop 'organization' here. Cross-organization targeting applies to ` +
332+
`'position', 'org_membership_level', 'department' and 'expression' approvers.`,
333+
});
334+
}
307335
}
308336

309337
// Empty-slate dead-end (#3424): when EVERY approver on the node routes to

0 commit comments

Comments
 (0)