diff --git a/.changeset/adr-0105-d9-cross-org-approver-targeting.md b/.changeset/adr-0105-d9-cross-org-approver-targeting.md new file mode 100644 index 0000000000..aaf2820581 --- /dev/null +++ b/.changeset/adr-0105-d9-cross-org-approver-targeting.md @@ -0,0 +1,53 @@ +--- +"@objectstack/plugin-approvals": minor +"@objectstack/spec": minor +"@objectstack/lint": minor +--- + +feat(approvals): cross-organization approver targeting — a plant document can +require a group-side sign-off (ADR-0105 D9) + +One organization id used to decide 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', )` +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: + +```yaml +approvers: + - { type: position, value: plant_manager, group: plant } + - { type: position, value: cfo, organization: $root, group: finance } +behavior: per_group +``` + +- **`$root` / `$parent`** walk D6's `parent_organization_id` tree, so the two + common intents need **no deployment knowledge** — flow metadata is portable + across environments while organization ids are minted per deployment. A slug + covers what the symbols cannot, notably a **sibling** organization (a + shared-services centre approving payables for every plant). +- Declared **per approver**, so one node can require a plant manager and a + group CFO in parallel. A node-level form cannot express that without + splitting into serial nodes, which changes the semantics. +- **Bounded, not free:** the target must share a `parent_organization_id` root + with the request's organization. The rule reads only the organization tree — + never the submitter — so one flow routes identically for everyone. + +Everything else fails loudly rather than quietly: + +- a non-`group` posture **refuses** the declaration (a `group` → `isolated` + migration must not silently reroute approvals); +- an approver type with no org-scoped directory (`user` / `field` / `manager` / + `team`) refuses it too, and a new `approval-approver-cross-org-unsupported` + lint catches that at author time; +- 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, so the node's existing + `onEmptyApprovers` policy takes over instead of leaving an unopenable task. + +Nothing changes for an approver without `organization`: same resolution, same +queries, no extra reads. diff --git a/content/docs/automation/approvals.mdx b/content/docs/automation/approvals.mdx index 4d09b3aa09..b52746d5e1 100644 --- a/content/docs/automation/approvals.mdx +++ b/content/docs/automation/approvals.mdx @@ -173,6 +173,68 @@ submit: with `behavior: 'per_group'`, each intermediate value (e.g. each returned department) forms its own sign-off group. +## Approving across organizations (ADR-0105 D9) [#cross-org-approvers] + + +Requires the `group` tenancy posture. Under `single` or `isolated` the runtime +**refuses** an approver that declares `organization`, rather than ignoring it. + + +A group-shaped deployment routinely needs a plant's document signed off by +someone at the group: the purchase order lives in the **plant** organization, +but the CFO holds her `cfo` position in the **group** organization. By default +an approver is resolved against the request's own organization, so `cfo` would +match nobody there. + +Declare which organization's directory resolves that approver: + +```typescript +config: { + approvers: [ + { type: 'position', value: 'plant_manager', group: 'plant' }, + { type: 'position', value: 'cfo', organization: '$root', group: 'finance' }, + ], + behavior: 'per_group', // one plant sign-off AND one group sign-off +} +``` + +`organization` takes a **symbol** or an organization **slug**: + +| Value | Resolves to | +| :--- | :--- | +| `$root` | the group organization — climbs `parent_organization_id` to the top | +| `$parent` | exactly one level up (division sign-off in a three-tier group) | +| a slug, e.g. `acme-ssc` | that organization — for a **sibling**, such as a shared-services centre approving payables for every plant | + +Prefer the symbols. Flow metadata is portable across environments while +organization ids are minted per deployment, so `$root` says "the group" without +naming anything deployment-specific. An organization **id** is rejected: pass +the slug. + +Three things this deliberately does **not** do: + +- **It does not move the request.** Only the approver lookup changes — the + request, its audit trail, and its inbox rows stay in the plant's + organization. +- **It is not a route to any tenant.** The target must share a + `parent_organization_id` root with the request's organization, so approval + routes *within one group*. The rule reads only the organization tree, never + the submitter, so a flow routes identically for everyone. +- **It does not apply to every approver kind.** `user`, `field`, `manager` and + `team` name people without consulting an organization directory; + `organization` on those is refused at runtime and flagged by `os lint` + (`approval-approver-cross-org-unsupported`). + + +A cross-organization approver must also be able to **read** the request. The +request stays in the plant's organization, so a group-side approver reaches it +only if she also holds a membership there — the usual group shape is that group +staff are members of every plant while holding their positions at the group. +Approvers without that membership are dropped from the slate with a warning +naming them, and the node's `onEmptyApprovers` policy takes over — better an +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 @@ -230,7 +292,10 @@ hierarchy. `queue` still parses so stored flows keep loading, but it is **not implemented** by the runtime and is no longer offered for authoring (#3508) — a queue entry resolves to nobody. Route to a `team`, `department`, or `position` instead. `field`, `manager`, and `expression` resolve against the record's -**live** state at node entry (#3447). An entry that resolves +**live** state at node entry (#3447). `position`, `department`, +`org_membership_level` and `expression` may additionally name which +organization's directory resolves them — see +[Approving across organizations](#cross-org-approvers). 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. diff --git a/content/docs/references/automation/approval.mdx b/content/docs/references/automation/approval.mdx index 7521b9b912..cda235a42d 100644 --- a/content/docs/references/automation/approval.mdx +++ b/content/docs/references/automation/approval.mdx @@ -64,6 +64,7 @@ const result = ApprovalDecision.parse(data); | **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. | --- @@ -74,7 +75,7 @@ const result = ApprovalDecision.parse(data); | Property | Type | Required | Description | | :--- | :--- | :--- | :--- | -| **approvers** | `{ type: Enum<'manager' \| 'position' \| 'department' \| 'team' \| 'field' \| 'expression' \| 'org_membership_level' \| 'role' \| 'user' \| 'queue'>; value?: string; resolveAs?: Enum<'user' \| 'department' \| 'position' \| 'team'>; group?: string }[]` | ✅ | Allowed approvers for this node | +| **approvers** | `{ type: Enum<'manager' \| 'position' \| 'department' \| 'team' \| 'field' \| 'expression' \| 'org_membership_level' \| 'role' \| 'user' \| 'queue'>; 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 | diff --git a/docs/adr/0105-group-tenancy-posture-and-first-class-org-scope.md b/docs/adr/0105-group-tenancy-posture-and-first-class-org-scope.md index 34e2d617fe..31824bad83 100644 --- a/docs/adr/0105-group-tenancy-posture-and-first-class-org-scope.md +++ b/docs/adr/0105-group-tenancy-posture-and-first-class-org-scope.md @@ -249,14 +249,53 @@ the `single`-posture deployment (plant-autonomous member admission) and is the natural admission UX in `group` posture. **D9 — Cross-org approval targeting.** -An approval chain node may name a **target organization** for approver -resolution (default: the request's org, today's behavior at -`approval-node.ts:118`). A plant document's escalation step declares -`organization: ` and resolves group-side position holders. Reads of -the request by those approvers are covered by D2 (membership union) in `group` -posture; in `isolated` posture this decision does not apply (cross-org +An approval **approver** may name a **target organization** for its resolution +(default: the request's org, today's behavior). A plant document's escalation +step declares `organization: $root` and resolves group-side position holders. +Reads of the request by those approvers are covered by D2 (membership union) in +`group` posture; in `isolated` posture this decision does not apply (cross-org approval there remains mirroring via system context, cloud #2937 contract). +*Amended during implementation (#3812). Four points the original text left +open, each settled from what the code and the authoring model actually +require:* + +1. *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 (`behavior: 'per_group'`). Expressing it as two serial nodes + changes the semantics (parallel co-sign becomes sequential approval), so the + node-level form distorts the model rather than merely inconveniencing the + author. A node-level default is a strict special case of the per-approver + form and remains addable later as sugar; the reverse is not true. +2. *Symbolic references first: `$root`, `$parent`.* Flow metadata is portable + across environments; an organization id is data minted per deployment. A + literal id in a flow is therefore unportable by construction, and an AI + author cannot know one at authoring time. The symbols express the two common + intents against D6's tree with **zero deployment knowledge**. A slug covers + what they cannot — notably a **sibling** organization (a shared-services + centre approving payables for every plant). +3. *Legality is "shares a `parent_organization_id` root", not "is an + ancestor".* The sibling case above is first-class, as is downward targeting. + The rule depends only on the organization tree and **never on the + submitter**, so one flow routes identically for everyone — which is what + makes a routing bug reproducible. A deployment with no grouping metadata + gets a refusal naming D6, not a silent pass. +4. *Non-`group` postures **refuse** at runtime rather than ignoring.* Posture is + environment configuration, so the same portable metadata may be deployed + into any posture and no static check can see which. Silently ignoring the + declaration would let a `group` → `isolated` migration reroute approvals + with no signal — an audit event, not a config detail. + +*Two consequences worth stating, both enforced:* an approver type that resolves +no org-scoped directory (`user` / `field` / `manager` / `team`) **refuses** the +declaration instead of ignoring it (an author who wrote it believed it did +something); and a targeted approver who holds no membership in the request's +organization is **dropped with a warning** naming them, because D2's union +would otherwise hide the request from someone the router had already committed +to — routing succeeds, the inbox row is written, and the approver opens a task +she cannot open. Dropping converts that silent dead-end into the node's +existing `onEmptyApprovers` policy. + **D10 — Layered master data (group template + org override).** A spec-level pattern for the SAP material-master / 用友-金蝶 distribution shape: an object may declare layered governance — group-level template rows diff --git a/packages/lint/src/index.ts b/packages/lint/src/index.ts index 4a16970fce..466c5d5681 100644 --- a/packages/lint/src/index.ts +++ b/packages/lint/src/index.ts @@ -135,6 +135,7 @@ export { APPROVAL_EXPRESSION_INVALID, APPROVAL_EXPRESSION_NO_EMPTY_POLICY, APPROVAL_DECISION_OUTPUTS_RESERVED, + APPROVAL_APPROVER_CROSS_ORG_UNSUPPORTED, } from './validate-approval-approvers.js'; export type { ApprovalApproverFinding, ApprovalApproverSeverity } from './validate-approval-approvers.js'; diff --git a/packages/lint/src/validate-approval-approvers.test.ts b/packages/lint/src/validate-approval-approvers.test.ts index 492dac6e9a..deb74d516c 100644 --- a/packages/lint/src/validate-approval-approvers.test.ts +++ b/packages/lint/src/validate-approval-approvers.test.ts @@ -12,6 +12,7 @@ import { APPROVAL_EXPRESSION_INVALID, APPROVAL_EXPRESSION_NO_EMPTY_POLICY, APPROVAL_DECISION_OUTPUTS_RESERVED, + APPROVAL_APPROVER_CROSS_ORG_UNSUPPORTED, } from './validate-approval-approvers.js'; function stackWithApprovers(approvers: unknown[]): Record { @@ -334,3 +335,39 @@ describe('expression approvers (#3447 P2)', () => { }))).toEqual([]); }); }); + +describe('cross-organization targeting (ADR-0105 D9)', () => { + it('is clean when `organization` sits on an org-scoped approver type', () => { + // The whole point of D9: a group CFO resolved against the group directory. + const findings = validateApprovalApprovers( + stackWithApprovers([{ type: 'position', value: 'cfo', organization: '$root' }]), + ); + expect(findings.filter(f => f.rule === APPROVAL_APPROVER_CROSS_ORG_UNSUPPORTED)).toEqual([]); + }); + + it('errors when `organization` sits on a type with no organization directory', () => { + // `user` names a person outright — the declaration cannot narrow anything, + // and the runtime refuses it. Catch it at author time instead. + const findings = validateApprovalApprovers( + stackWithApprovers([{ type: 'user', value: 'u1', organization: '$root' }]), + ); + const f = findings.find(x => x.rule === APPROVAL_APPROVER_CROSS_ORG_UNSUPPORTED); + expect(f?.severity).toBe('error'); + expect(f?.path).toBe('flows[0].nodes[1].config.approvers[0].organization'); + expect(f?.hint).toMatch(/position.*org_membership_level.*department.*expression/); + }); + + it('errors for `team` too — team membership carries no organization', () => { + const findings = validateApprovalApprovers( + stackWithApprovers([{ type: 'team', value: 't1', organization: 'acme-ssc' }]), + ); + expect(findings.some(f => f.rule === APPROVAL_APPROVER_CROSS_ORG_UNSUPPORTED)).toBe(true); + }); + + it('stays silent when no organization is declared — the default path', () => { + const findings = validateApprovalApprovers( + stackWithApprovers([{ type: 'user', value: 'u1' }]), + ); + expect(findings.filter(f => f.rule === APPROVAL_APPROVER_CROSS_ORG_UNSUPPORTED)).toEqual([]); + }); +}); diff --git a/packages/lint/src/validate-approval-approvers.ts b/packages/lint/src/validate-approval-approvers.ts index ffaf625d88..e0cd38ef16 100644 --- a/packages/lint/src/validate-approval-approvers.ts +++ b/packages/lint/src/validate-approval-approvers.ts @@ -24,6 +24,7 @@ * | approval-expression-invalid | error/info | #3447 P2 closed-root expressions | * | approval-expression-no-empty-policy | info | #3447 P2 empty-slate policy | * | approval-decision-outputs-reserved | error | #3447 P2 resume envelope | + * | approval-approver-cross-org-unsupported | error | ADR-0105 D9 targeting | * * The first two are mutually exclusive by construction — a bad *value* wins, * because its fix (`position`) differs from the deprecation's fix @@ -42,6 +43,7 @@ import { APPROVAL_NODE_TYPE, DEPRECATED_APPROVER_TYPES, APPROVER_VALUE_BINDINGS, + approverTypeIsOrgScoped, canonicalApproverType, normalizeDecisionOutputs, } from '@objectstack/spec/automation'; @@ -57,6 +59,7 @@ export const APPROVAL_APPROVERS_MAY_RESOLVE_EMPTY = 'approval-approvers-may-reso export const APPROVAL_EXPRESSION_INVALID = 'approval-expression-invalid'; export const APPROVAL_EXPRESSION_NO_EMPTY_POLICY = 'approval-expression-no-empty-policy'; export const APPROVAL_DECISION_OUTPUTS_RESERVED = 'approval-decision-outputs-reserved'; +export const APPROVAL_APPROVER_CROSS_ORG_UNSUPPORTED = 'approval-approver-cross-org-unsupported'; /** * The CLOSED root set an `expression` approver may reference (#3447 P2) — @@ -304,6 +307,31 @@ export function validateApprovalApprovers(stack: AnyRec): ApprovalApproverFindin `Queue approvers need a real ownership-queue implementation before they take effect.`, }); } + + // [ADR-0105 D9] Cross-organization targeting on a type that has no + // organization-scoped directory. `user` / `field` / `manager` name a + // person outright and `team` membership carries no organization, so the + // declaration cannot narrow anything — it is a misunderstanding of what + // the field does, and the runtime refuses it. Error, not warning: this + // is a certain authoring mistake with a certain fix, and letting it + // reach the runtime turns author time into an incident. + const declaredOrg = (a as AnyRec).organization; + if (typeof declaredOrg === 'string' && declaredOrg.trim() !== '' + && ApproverType.options.includes(canonical as never) + && !approverTypeIsOrgScoped(canonical)) { + findings.push({ + severity: 'error', + rule: APPROVAL_APPROVER_CROSS_ORG_UNSUPPORTED, + where, + path: `${path}.organization`, + message: + `approver type '${type}' does not resolve through an organization directory, so ` + + `'organization: ${declaredOrg}' has no effect (ADR-0105 D9) — the runtime refuses it.`, + hint: + `Drop 'organization' here. Cross-organization targeting applies to ` + + `'position', 'org_membership_level', 'department' and 'expression' approvers.`, + }); + } } // Empty-slate dead-end (#3424): when EVERY approver on the node routes to diff --git a/packages/plugins/plugin-approvals/src/approval-service.ts b/packages/plugins/plugin-approvals/src/approval-service.ts index dc591e072d..a36eb31ffc 100644 --- a/packages/plugins/plugin-approvals/src/approval-service.ts +++ b/packages/plugins/plugin-approvals/src/approval-service.ts @@ -3,6 +3,7 @@ import { createHash, randomBytes } from 'node:crypto'; import { APPROVAL_BRANCH_LABELS, + approverTypeIsOrgScoped, canonicalApproverType, normalizeDecisionOutputs, type ApprovalNodeConfig, @@ -33,6 +34,12 @@ import type { } from '@objectstack/spec/contracts'; import { isFileIdToken } from '@objectstack/spec/data'; import { isGrantActive } from '@objectstack/core'; +import { + filterApproversWhoCanRead, + resolveApproverDirectoryOrg, + type ApproverOrgScopeDeps, + type ApproverOrgScopeEngine, +} from './approver-org-scope.js'; /** * Node-era approval runtime (ADR-0019). @@ -416,6 +423,14 @@ export interface ApprovalServiceOptions { * the Console and IM webviews; outbound email needs the absolute form. */ publicBaseUrl?: string; + /** + * [ADR-0105 D9] The tenancy posture in force. Cross-organization approver + * targeting is a `group`-posture capability; the resolver refuses the + * declaration under any other posture rather than silently ignoring it. + * Absent (a stack booted with no tenancy service) reads as "unknown" and the + * guard stands down. + */ + tenancyPosture?: () => string | undefined; } export class ApprovalService implements IApprovalService { @@ -425,6 +440,7 @@ export class ApprovalService implements IApprovalService { private automation?: ApprovalResumeSurface; private messaging?: ApprovalMessagingSurface; private publicBaseUrl: string; + private tenancyPosture?: () => string | undefined; constructor(opts: ApprovalServiceOptions) { this.engine = opts.engine; @@ -433,6 +449,36 @@ export class ApprovalService implements IApprovalService { this.automation = opts.automation; this.messaging = opts.messaging; this.publicBaseUrl = (opts.publicBaseUrl ?? '').replace(/\/$/, ''); + this.tenancyPosture = opts.tenancyPosture; + } + + /** Attach (or replace) the ADR-0105 D9 posture provider. */ + attachTenancyPosture(provider: () => string | undefined): void { + this.tenancyPosture = provider; + } + + /** Deps bundle for the ADR-0105 D9 org-scope helpers. */ + private get orgScopeDeps(): ApproverOrgScopeDeps { + return { + engine: this.engine as unknown as ApproverOrgScopeEngine, + posture: this.tenancyPosture, + logger: this.logger, + }; + } + + /** + * [ADR-0105 D9] Which organization's directory resolves ONE approver spec. + * Absent declaration ⇒ the request's own organization (unchanged, no reads). + */ + private async directoryOrgFor(a: any, requestOrgId: string | null | undefined): Promise { + const rawType = String(a?.type ?? ''); + return resolveApproverDirectoryOrg( + this.orgScopeDeps, + a?.organization, + requestOrgId, + rawType, + approverTypeIsOrgScoped(rawType), + ); } /** Attach (or replace) the automation surface used to resume flow runs. */ @@ -740,18 +786,35 @@ export class ApprovalService implements IApprovalService { } return out; } + // [ADR-0105 D9] WHERE this approver is looked up — the request's own + // organization unless the spec targets another one in the same group. + // Resolution failures propagate: they are routing bugs, and this call is + // OUTSIDE the swallowing try below on purpose (see the catch's comment). + const directoryOrg = await this.directoryOrgFor(a, organizationId); + const crossOrg = directoryOrg !== organizationId; + // A cross-org slate is filtered to the people who can actually READ the + // request (D2 union); same-org routing is untouched and does no extra read. + const bounded = async (users: string[]): Promise => ( + crossOrg + ? filterApproversWhoCanRead(this.orgScopeDeps, users, organizationId, { + approverType: type, value: a.value != null ? String(a.value) : undefined, + directoryOrgId: directoryOrg, + }) + : users + ); + try { if (type === 'team') { const users = await this.expandTeamUsers(String(a.value)); if (users.length) return users; } else if (type === 'department' || type === 'business_unit' || type === 'bu') { - const users = await this.expandBusinessUnitUsers(String(a.value), organizationId); + const users = await bounded(await this.expandBusinessUnitUsers(String(a.value), directoryOrg)); if (users.length) return users; } else if (type === 'position') { - const users = await this.expandPositionUsers(String(a.value), organizationId); + const users = await bounded(await this.expandPositionUsers(String(a.value), directoryOrg)); if (users.length) return users; } else if (type === 'org_membership_level') { - const users = await this.expandMembershipTierUsers(String(a.value), organizationId); + const users = await bounded(await this.expandMembershipTierUsers(String(a.value), directoryOrg)); if (users.length) return users; } else if (type === 'manager' && record) { const subject = (record as any)[a.value] ?? (record as any).owner_id; @@ -760,7 +823,7 @@ export class ApprovalService implements IApprovalService { if (mgr) return this.applyOooDelegation(mgr, now, organizationId, substitutions); } } - } catch { /* fall through */ } + } catch { /* a directory lookup failed → fall through to the literal slot */ } // #3508: `queue` is declared-but-unenforced — there is no queue branch // above, so a queue approver always lands here and the `queue:` slot // routes to nobody. The spec marks it non-authorable @@ -893,12 +956,18 @@ export class ApprovalService implements IApprovalService { } return { slots, raw }; } + // [ADR-0105 D9] An expression that re-expands into a graph kind consults the + // same org-scoped directories the static types do, so it honours the same + // targeting. Resolved once for the whole slate, before the per-value loop — + // the declaration is a property of the spec, not of what the CEL returned. + const directoryOrg = await this.directoryOrgFor(a, organizationId); + const crossOrg = directoryOrg !== organizationId; const slots: Array<{ id: string; subGroup: string }> = []; for (const key of raw) { let users: string[] = []; try { - if (resolveAs === 'department') users = await this.expandBusinessUnitUsers(key, organizationId); - else if (resolveAs === 'position') users = await this.expandPositionUsers(key, organizationId); + if (resolveAs === 'department') users = await this.expandBusinessUnitUsers(key, directoryOrg); + else if (resolveAs === 'position') users = await this.expandPositionUsers(key, directoryOrg); else if (resolveAs === 'team') users = await this.expandTeamUsers(key); else { throw new Error( @@ -910,6 +979,11 @@ export class ApprovalService implements IApprovalService { if (String(err?.message ?? '').startsWith('VALIDATION_FAILED')) throw err; users = []; } + if (crossOrg && users.length) { + users = await filterApproversWhoCanRead(this.orgScopeDeps, users, organizationId, { + approverType: resolveAs, value: key, directoryOrgId: directoryOrg, + }); + } if (!users.length) { slots.push({ id: `${resolveAs}:${key}`, subGroup: key }); continue; diff --git a/packages/plugins/plugin-approvals/src/approvals-plugin.ts b/packages/plugins/plugin-approvals/src/approvals-plugin.ts index f83b08bcdd..0b4ded6574 100644 --- a/packages/plugins/plugin-approvals/src/approvals-plugin.ts +++ b/packages/plugins/plugin-approvals/src/approvals-plugin.ts @@ -121,6 +121,20 @@ export class ApprovalsServicePlugin implements Plugin { engine: engine as ApprovalEngine, logger: ctx.logger, publicBaseUrl: this.options.publicBaseUrl, + // [ADR-0105 D9] Cross-organization approver targeting is a `group`-posture + // capability. Read LAZILY (not captured at start) because the tenancy + // service resolves its posture during its own start, which may not have + // run yet; an unresolvable posture reads as "unknown" and the guard + // stands down rather than refusing a legitimate flow on a minimal stack. + tenancyPosture: () => { + try { + const tenancy = ctx.getService<{ posture?: string }>('tenancy'); + const posture = tenancy?.posture; + return typeof posture === 'string' && posture ? posture : undefined; + } catch { + return undefined; + } + }, }); // Record lock: block edits to a record while it has a pending request. diff --git a/packages/plugins/plugin-approvals/src/approver-cross-org.integration.test.ts b/packages/plugins/plugin-approvals/src/approver-cross-org.integration.test.ts new file mode 100644 index 0000000000..6f449d3405 --- /dev/null +++ b/packages/plugins/plugin-approvals/src/approver-cross-org.integration.test.ts @@ -0,0 +1,179 @@ +// Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license. + +/** + * [ADR-0105 D9] Cross-organization approver targeting, through the SERVICE. + * + * `approver-org-scope.test.ts` owns the resolver's own rules. What only this + * level can show is that the resolved organization actually reaches the + * directory lookups — the wiring D9 is: one organization id used to decide + * three different things at once (where the request lives, where its inbox rows + * live, where its approvers are looked up), and only the third moves. + * + * The scenario is the one D9 exists for: a purchase order raised in PLANT A + * needs the group CFO, who holds `cfo` in the GROUP organization and would have + * matched nobody before. + */ + +import { describe, it, expect, beforeEach } from 'vitest'; +import { ApprovalService } from './approval-service.js'; + +interface Row { [k: string]: any } + +function makeEngine(seed: Record = {}) { + const tables: Record = { ...seed }; + const ensure = (n: string) => (tables[n] ??= []); + const matches = (row: Row, filter: any): boolean => { + if (!filter || typeof filter !== 'object') return true; + for (const [k, v] of Object.entries(filter)) { + if (k === '$or') { + if (!(v as any[]).some((sub) => matches(row, sub))) return false; + continue; + } + const rv = row[k]; + if (v != null && typeof v === 'object' && '$in' in (v as any)) { + if (!(v as any).$in.includes(rv)) return false; + continue; + } + if (v != null && typeof v === 'object' && '$ne' in (v as any)) { + if (rv === (v as any).$ne) return false; + continue; + } + if (rv !== v) return false; + } + return true; + }; + return { + _tables: tables, + async find(object: string, options?: any) { + return ensure(object).filter((r) => matches(r, options?.filter ?? options?.where)); + }, + async insert(object: string, data: Row) { ensure(object).push({ ...data }); return { ...data }; }, + async update(_o: string, _w: any, _d: any) { return {}; }, + async delete() { return {}; }, + async count(object: string) { return ensure(object).length; }, + registerHook() { /* no-op */ }, + unregisterHooksByPackage() { /* no-op */ }, + }; +} + +/** Plant A sits under the group; the CFO's position lives in the group org. */ +const SEED = () => ({ + sys_organization: [ + { id: 'o_group', slug: 'acme-group', parent_organization_id: null }, + { id: 'o_plant', slug: 'acme-plant-a', parent_organization_id: 'o_group' }, + ], + sys_user_position: [ + { id: 'up1', user_id: 'u_cfo', position: 'cfo', organization_id: 'o_group' }, + { id: 'up2', user_id: 'u_plant_mgr', position: 'plant_manager', organization_id: 'o_plant' }, + ], + sys_member: [ + // The intended group shape: group staff hold a membership in every plant + // (so D2's union lets them READ the request) while their POSITION lives in + // the group organization. + { id: 'm1', user_id: 'u_cfo', organization_id: 'o_plant', role: 'member' }, + { id: 'm2', user_id: 'u_plant_mgr', organization_id: 'o_plant', role: 'member' }, + ], +}); + +const CTX = { userId: 'u_submitter', organizationId: 'o_plant', positions: [], permissions: [] } as any; + +function openInput(approvers: any[], extra: Record = {}) { + return { + object: 'purchase_order', + recordId: 'po1', + runId: 'run_1', + nodeId: 'group_signoff', + flowName: 'po_approval', + config: { approvers, behavior: 'unanimous' as const, lockRecord: false }, + record: { id: 'po1', amount: 500000 }, + ...extra, + }; +} + +describe('ADR-0105 D9 — cross-org approver targeting through ApprovalService', () => { + let engine: ReturnType; + let svc: ApprovalService; + let n = 0; + + beforeEach(() => { + engine = makeEngine(SEED()); + n = 0; + svc = new ApprovalService({ + engine: engine as any, + clock: { now: () => new Date(1767000000000 + (n++) * 1000) }, + tenancyPosture: () => 'group', + }); + }); + + it('without targeting, a group position matches NOBODY — the gap D9 closes', async () => { + // The `cfo` position exists, but in the group org; the request is Plant A's. + // Pre-D9 this was the only possible outcome, and it was silent. + const req = await svc.openNodeRequest(openInput([{ type: 'position', value: 'cfo' }]), CTX); + expect(req.pending_approvers).toEqual(['position:cfo']); // the dead literal slot + }); + + it('`organization: $root` resolves the CFO against the GROUP directory', async () => { + const req = await svc.openNodeRequest( + openInput([{ type: 'position', value: 'cfo', organization: '$root' }]), + CTX, + ); + expect(req.pending_approvers).toEqual(['u_cfo']); + }); + + it('the request and its inbox rows still belong to the REQUEST org — only the lookup moved', async () => { + await svc.openNodeRequest( + openInput([{ type: 'position', value: 'cfo', organization: '$root' }]), + CTX, + ); + // This is the whole point of the split: targeting must not relocate the + // request into the approver's organization, or the plant would lose its own + // audit trail to the group. + expect(engine._tables['sys_approval_request'][0].organization_id).toBe('o_plant'); + expect(engine._tables['sys_approval_approver'][0].organization_id).toBe('o_plant'); + expect(engine._tables['sys_approval_action'][0].organization_id).toBe('o_plant'); + }); + + it('mixes a plant approver and a group approver in ONE node — why targeting is per-approver', async () => { + // A node-level declaration could not express this: the plant manager and the + // group CFO sign off in parallel, each resolved in a different directory. + const req = await svc.openNodeRequest( + openInput([ + { type: 'position', value: 'plant_manager', group: 'plant' }, + { type: 'position', value: 'cfo', organization: '$root', group: 'finance' }, + ]), + CTX, + ); + expect(new Set(req.pending_approvers)).toEqual(new Set(['u_plant_mgr', 'u_cfo'])); + }); + + it('drops a targeted approver who could not READ the request, leaving the empty-slate path', async () => { + // Same flow, but the CFO holds no membership in Plant A — D2's union wall + // would hide the request from her. Better an empty slate (which the node has + // an `onEmptyApprovers` policy for) than a task she cannot open. + engine._tables['sys_member'] = engine._tables['sys_member'].filter((m) => m.user_id !== 'u_cfo'); + const req = await svc.openNodeRequest( + openInput([{ type: 'position', value: 'cfo', organization: '$root' }]), + CTX, + ); + expect(req.pending_approvers).toEqual(['position:cfo']); + }); + + it('refuses a target outside the group — loudly, and no request is created', async () => { + engine._tables['sys_organization'].push({ id: 'o_rival', slug: 'rival-co', parent_organization_id: null }); + await expect( + svc.openNodeRequest(openInput([{ type: 'position', value: 'cfo', organization: 'rival-co' }]), CTX), + ).rejects.toThrow(/VALIDATION_FAILED.*not in the same group/); + expect(engine._tables['sys_approval_request'] ?? []).toHaveLength(0); + }); + + it('refuses under a non-group posture — a posture migration cannot silently reroute', async () => { + const isolated = new ApprovalService({ + engine: engine as any, + clock: { now: () => new Date(1767000000000) }, + tenancyPosture: () => 'isolated', + }); + await expect( + isolated.openNodeRequest(openInput([{ type: 'position', value: 'cfo', organization: '$root' }]), CTX), + ).rejects.toThrow(/VALIDATION_FAILED.*'group' tenancy posture/); + }); +}); diff --git a/packages/plugins/plugin-approvals/src/approver-org-scope.test.ts b/packages/plugins/plugin-approvals/src/approver-org-scope.test.ts new file mode 100644 index 0000000000..8de069a60b --- /dev/null +++ b/packages/plugins/plugin-approvals/src/approver-org-scope.test.ts @@ -0,0 +1,201 @@ +// Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license. + +/** + * [ADR-0105 D9] Cross-organization approver targeting. + * + * The property that matters is that this is a route WITHIN one group, not a + * channel to an arbitrary tenant — so every test here is about a boundary + * holding or a failure being LOUD. The thing D9 exists to prevent is silence: + * an approval that reads as "escalate to group" but quietly resolves inside the + * plant, or one that routes to someone the D2 wall then hides the request from. + */ + +import { describe, it, expect, vi } from 'vitest'; +import { + filterApproversWhoCanRead, + resolveApproverDirectoryOrg, + type ApproverOrgScopeDeps, +} from './approver-org-scope.js'; + +/** + * A three-tier group: group → division → plant, plus a shared-services SIBLING + * of the plant, plus an organization in a DIFFERENT group entirely. + */ +const ORGS: Record = { + o_group: { id: 'o_group', slug: 'acme-group', parent_organization_id: null }, + o_div: { id: 'o_div', slug: 'acme-north', parent_organization_id: 'o_group' }, + o_plant: { id: 'o_plant', slug: 'acme-plant-a', parent_organization_id: 'o_div' }, + o_ssc: { id: 'o_ssc', slug: 'acme-ssc', parent_organization_id: 'o_group' }, + o_other: { id: 'o_other', slug: 'rival-co', parent_organization_id: null }, + o_lone: { id: 'o_lone', slug: 'lone-co', parent_organization_id: null }, +}; + +function makeDeps(over: Partial & { members?: Array<{ user_id: string; organization_id: string }> } = {}): ApproverOrgScopeDeps & { warn: any } { + const warn = vi.fn(); + const members = over.members ?? []; + const engine = { + find: vi.fn(async (object: string, opts: any) => { + const f = opts?.filter ?? {}; + if (object === 'sys_organization') { + const rows = Object.values(ORGS).filter((o) => + (f.id === undefined || o.id === f.id) && (f.slug === undefined || o.slug === f.slug)); + return rows; + } + if (object === 'sys_member') { + const wanted: string[] = f.user_id?.$in ?? []; + return members.filter((m) => m.organization_id === f.organization_id && wanted.includes(m.user_id)); + } + return []; + }), + }; + return { engine, logger: { warn }, posture: () => 'group', ...over, warn } as any; +} + +const resolve = ( + deps: ApproverOrgScopeDeps, + declaration: string | undefined, + requestOrg: string | null, + type = 'position', + orgScoped = true, +) => resolveApproverDirectoryOrg(deps, declaration, requestOrg, type, orgScoped); + +describe('resolveApproverDirectoryOrg — the default path is untouched', () => { + it('returns the request org and reads NOTHING when no organization is declared', async () => { + const deps = makeDeps(); + await expect(resolve(deps, undefined, 'o_plant')).resolves.toBe('o_plant'); + // The overwhelmingly common case must not cost a query. + expect((deps.engine.find as any)).not.toHaveBeenCalled(); + }); + + it('passes a null request org straight through — a single-org deployment is unaffected', async () => { + const deps = makeDeps(); + await expect(resolve(deps, undefined, null)).resolves.toBeNull(); + }); +}); + +describe('resolveApproverDirectoryOrg — symbols resolve against the D6 tree', () => { + it('$root climbs to the group organization, not just one level', async () => { + // o_plant → o_div → o_group. A one-level implementation would answer o_div. + await expect(resolve(makeDeps(), '$root', 'o_plant')).resolves.toBe('o_group'); + }); + + it('$parent stops at exactly one level — division sign-off in a three-tier group', async () => { + await expect(resolve(makeDeps(), '$parent', 'o_plant')).resolves.toBe('o_div'); + }); + + it('$root on an organization with no lineage FAILS instead of silently self-targeting', async () => { + // Returning o_lone would make "escalate to group" mean "approve in place" — + // the exact silent misrouting D9 exists to prevent. + await expect(resolve(makeDeps(), '$root', 'o_lone')).rejects.toThrow( + /VALIDATION_FAILED.*no 'parent_organization_id' lineage/, + ); + }); + + it('$parent at the root of a group fails rather than resolving to nothing', async () => { + await expect(resolve(makeDeps(), '$parent', 'o_group')).rejects.toThrow(/VALIDATION_FAILED.*no parent/); + }); +}); + +describe('resolveApproverDirectoryOrg — a slug names one organization, bounded by the group', () => { + it('accepts a SIBLING in the same group — the shared-services-centre shape', async () => { + // o_ssc is not an ancestor of o_plant; it shares the root. This is why the + // rule is "shares a root", not "is an ancestor". + await expect(resolve(makeDeps(), 'acme-ssc', 'o_plant')).resolves.toBe('o_ssc'); + }); + + it('refuses an organization in a DIFFERENT group — this is not a channel to any tenant', async () => { + await expect(resolve(makeDeps(), 'rival-co', 'o_plant')).rejects.toThrow( + /VALIDATION_FAILED.*not in the same group/, + ); + }); + + it('refuses an unknown slug and says an id is not accepted', async () => { + // The most likely authoring mistake is pasting an organization id, which is + // per-deployment and would make the flow unportable. + await expect(resolve(makeDeps(), 'o_plant', 'o_plant')).rejects.toThrow( + /VALIDATION_FAILED.*never an id/, + ); + }); +}); + +describe('resolveApproverDirectoryOrg — the guards', () => { + it('refuses under a non-group posture instead of silently ignoring the declaration', async () => { + // A deployment migrating group → isolated must not quietly reroute its + // approvals; that is an audit event, not a config detail. + const deps = makeDeps({ posture: () => 'isolated' }); + await expect(resolve(deps, '$root', 'o_plant')).rejects.toThrow( + /VALIDATION_FAILED.*requires the 'group' tenancy posture.*isolated/, + ); + }); + + it('stands down when the posture is unknown — a minimal stack is not broken by a guard it cannot answer', async () => { + const deps = makeDeps({ posture: () => undefined }); + await expect(resolve(deps, '$root', 'o_plant')).resolves.toBe('o_group'); + }); + + it('refuses the declaration on an approver type that has no organization directory', async () => { + // `user` names a person outright; an `organization` on it would have no + // effect. An author who wrote it believed it did something. + await expect(resolve(makeDeps(), '$root', 'o_plant', 'user', false)).rejects.toThrow( + /VALIDATION_FAILED.*would have no effect/, + ); + }); + + it('refuses when the request carries no organization at all', async () => { + await expect(resolve(makeDeps(), '$root', null)).rejects.toThrow( + /VALIDATION_FAILED.*carries no organization/, + ); + }); + + it('survives a cycle in the grouping metadata instead of looping forever', async () => { + const deps = makeDeps(); + (deps.engine.find as any) = vi.fn(async (object: string, opts: any) => { + if (object !== 'sys_organization') return []; + const id = opts?.filter?.id; + // a → b → a + if (id === 'a') return [{ id: 'a', slug: 'a', parent_organization_id: 'b' }]; + if (id === 'b') return [{ id: 'b', slug: 'b', parent_organization_id: 'a' }]; + return []; + }); + // Terminates; the cycle is broken and whatever it settles on is bounded. + await expect(resolve(deps, '$parent', 'a')).resolves.toBe('b'); + }); +}); + +describe('filterApproversWhoCanRead — routing to someone the wall hides is a silent failure', () => { + it('keeps approvers who hold a membership in the REQUEST org', async () => { + const deps = makeDeps({ members: [{ user_id: 'u_cfo', organization_id: 'o_plant' }] }); + const kept = await filterApproversWhoCanRead(deps, ['u_cfo'], 'o_plant', { + approverType: 'position', value: 'cfo', directoryOrgId: 'o_group', + }); + expect(kept).toEqual(['u_cfo']); + expect(deps.warn).not.toHaveBeenCalled(); + }); + + it('drops an approver with no membership there, and says exactly why', async () => { + // She holds `cfo` in the group org but no membership in the plant, so D2's + // union would hide the request from her: routing succeeds, then she opens a + // task she cannot open. Convert that into the empty-slate path the node + // already has a policy for. + const deps = makeDeps({ members: [{ user_id: 'u_ok', organization_id: 'o_plant' }] }); + const kept = await filterApproversWhoCanRead(deps, ['u_ok', 'u_no_membership'], 'o_plant', { + approverType: 'position', value: 'cfo', directoryOrgId: 'o_group', + }); + expect(kept).toEqual(['u_ok']); + expect(deps.warn).toHaveBeenCalledTimes(1); + const msg = String(deps.warn.mock.calls[0][0]); + expect(msg).toMatch(/u_no_membership|cross-organization approver/); + expect(msg).toMatch(/Grant those users a membership|retarget/); + }); + + it('does NOT empty a live slate when membership is unreadable', async () => { + // An infrastructure hiccup must not silently unstaff a pending approval: + // routing to someone who may not see it is recoverable, emptying is not. + const deps = makeDeps(); + (deps.engine.find as any) = vi.fn(async () => { throw new Error('driver hiccup'); }); + const kept = await filterApproversWhoCanRead(deps, ['u1', 'u2'], 'o_plant', { + approverType: 'position', directoryOrgId: 'o_group', + }); + expect(kept).toEqual(['u1', 'u2']); + }); +}); diff --git a/packages/plugins/plugin-approvals/src/approver-org-scope.ts b/packages/plugins/plugin-approvals/src/approver-org-scope.ts new file mode 100644 index 0000000000..a55750460a --- /dev/null +++ b/packages/plugins/plugin-approvals/src/approver-org-scope.ts @@ -0,0 +1,261 @@ +// Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license. + +/** + * [ADR-0105 D9] Cross-organization approver targeting — resolving WHICH + * organization's directory an approver is looked up in. + * + * One organization id used to do three jobs 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. Binding all three together meant + * `expandPositionUsers('cfo', )` matched nobody and the slot fell into + * `onEmptyApprovers` — a group escalation could not be expressed at all. + * + * This module resolves only the third job. The request keeps living in its own + * organization, and D2's membership union is what lets a group-side approver + * READ it — see `assertApproversCanRead` below for why that is a precondition + * worth checking rather than assuming. + */ + +/** Cycle guard for the `parent_organization_id` walk (mirrors the BU subtree walk). */ +const MAX_ORG_DEPTH = 32; + +const SYSTEM_CTX = { isSystem: true } as const; + +export interface ApproverOrgScopeEngine { + find(object: string, options: any): Promise; +} + +export interface ApproverOrgScopeDeps { + engine: ApproverOrgScopeEngine; + /** + * The tenancy posture in force, when the host could resolve one. `undefined` + * means "unknown" — a stack booted without the tenancy service — and is + * treated as permissive so a minimal test/embedded stack is not broken by a + * guard it cannot answer. + */ + posture?: () => string | undefined; + logger?: { warn?: (msg: any, ...rest: any[]) => void; debug?: (msg: any, ...rest: any[]) => void }; +} + +/** + * Every failure here THROWS `VALIDATION_FAILED`, matching how `expression` + * approvers already fail (#3447 P2): an approver declaration that cannot be + * resolved is a ROUTING BUG, never "condition not met". Silently falling back + * to the request's own organization would be the worst option available — a + * flow that reads as "escalate to group" would quietly approve inside the + * plant, and the deployment that changed posture would reroute its approvals + * with no signal at all. + * + * Messages name the offending value because their primary reader is the AI + * author fixing the flow on the next validate pass. + */ +function fail(message: string): never { + throw new Error(`VALIDATION_FAILED: ${message}`); +} + +async function findOrg( + engine: ApproverOrgScopeEngine, + where: Record, +): Promise { + try { + const rows = await engine.find('sys_organization', { + filter: where, + fields: ['id', 'slug', 'parent_organization_id'], + limit: 1, + context: SYSTEM_CTX, + } as any); + return Array.isArray(rows) && rows[0] ? rows[0] : null; + } catch { + return null; + } +} + +/** The id chain from `orgId` up to its root, inclusive. Fail-closed on cycles. */ +async function ancestorChain( + engine: ApproverOrgScopeEngine, + orgId: string, +): Promise { + const chain: string[] = []; + const seen = new Set(); + let cursor: string | null = orgId; + for (let depth = 0; cursor && depth < MAX_ORG_DEPTH; depth++) { + if (seen.has(cursor)) break; // cycle in the grouping metadata — stop, do not loop + seen.add(cursor); + chain.push(cursor); + const row = await findOrg(engine, { id: cursor }); + const parent = row?.parent_organization_id; + cursor = parent ? String(parent) : null; + } + return chain; +} + +/** + * Resolve an approver's `organization` declaration to a concrete organization + * id for directory lookups. Returns the request's own organization when the + * declaration is absent — the unchanged default path, which does no reads. + */ +export async function resolveApproverDirectoryOrg( + deps: ApproverOrgScopeDeps, + declaration: string | null | undefined, + requestOrgId: string | null | undefined, + approverType: string, + isOrgScopedType: boolean, +): Promise { + const declared = typeof declaration === 'string' ? declaration.trim() : ''; + if (!declared) return requestOrgId; + + // An `organization` on `user` / `field` / `manager` / `team` is not a + // narrower routing — those types never consult an org-scoped directory, so + // the declaration would have no effect. Refuse rather than ignore: a flow + // author who wrote it believed it did something. + if (!isOrgScopedType) { + fail( + `approver type '${approverType}' resolves people without an organization directory, ` + + `so 'organization: ${declared}' would have no effect — remove it ` + + `(ADR-0105 D9 applies to position / org_membership_level / department / expression)`, + ); + } + + // Posture guard. ADR-0105 D9 applies to `group` only: in `isolated`, cross-org + // approval remains system-context mirroring (cloud #2937 contract), and in + // `single` there is no second organization to target. Refusing here — rather + // than in a lint — is deliberate: posture is ENVIRONMENT configuration, so the + // same portable flow metadata may be deployed into any posture and no static + // check can see which. A deployment migrating group → isolated must fail + // loudly instead of silently rerouting its approvals. + const posture = deps.posture?.(); + if (posture && posture !== 'group') { + fail( + `cross-organization approver targeting ('organization: ${declared}') requires the ` + + `'group' tenancy posture; this deployment resolves '${posture}' (ADR-0105 D9)`, + ); + } + + const requestOrg = requestOrgId ? String(requestOrgId) : ''; + if (!requestOrg) { + fail( + `'organization: ${declared}' cannot be resolved for a request that carries no ` + + `organization — cross-organization targeting needs an organization to resolve from`, + ); + } + + const chain = await ancestorChain(deps.engine, requestOrg); + + if (declared === '$root') { + const root = chain[chain.length - 1]; + if (root === requestOrg && chain.length === 1) { + fail( + `'organization: $root' resolved to the request's own organization — this organization ` + + `has no 'parent_organization_id' lineage. Declare the group hierarchy (ADR-0105 D6) ` + + `or drop the targeting`, + ); + } + return root; + } + + if (declared === '$parent') { + const parent = chain[1]; + if (!parent) { + fail( + `'organization: $parent' has no parent to resolve to — the request's organization is ` + + `already the root of its group (ADR-0105 D6 'parent_organization_id')`, + ); + } + return parent; + } + + // A slug names one specific organization — the shape the symbols cannot + // express (a SIBLING, e.g. a shared-services centre approving payables for + // every plant). Slugs rather than ids because flow metadata is portable + // across environments and organization ids are minted per deployment. + const target = await findOrg(deps.engine, { slug: declared }); + if (!target?.id) { + fail( + `no organization with slug '${declared}' — 'organization' takes an organization SLUG ` + + `or a symbol ($root / $parent), never an id (ids are per-deployment, flows are portable)`, + ); + } + const targetId = String(target.id); + if (targetId === requestOrg) return targetId; + + // Legality: the target must share a grouping root with the request's + // organization. "Shares a root" rather than "is an ancestor" because the + // sibling case is a first-class use. The rule depends only on the + // organization tree — never on who submitted — so one flow routes identically + // for every submitter, which is what makes a routing bug reproducible. + const targetChain = await ancestorChain(deps.engine, targetId); + const targetRoot = targetChain[targetChain.length - 1]; + const requestRoot = chain[chain.length - 1]; + if (!targetRoot || !requestRoot || targetRoot !== requestRoot) { + fail( + `organization '${declared}' is not in the same group as the request's organization ` + + `— cross-organization approval routes WITHIN one group (they must share a ` + + `'parent_organization_id' root, ADR-0105 D6/D9)`, + ); + } + return targetId; +} + +/** + * Drop approvers who could not READ the request they were routed to, and say + * so. + * + * ADR-0105 D9 notes that reads by cross-org approvers "are covered by D2 + * (membership union)". True — but D2's union is `organization_id IN + * accessible_org_ids`, and the request row is stamped with the REQUEST's + * organization. So a group-side approver reaches it only if she also holds a + * membership there. That is the intended group shape (group staff are members + * of every plant while holding their positions in the group org — the shape the + * ADR-0105 acceptance dogfood already models), but nothing enforces it. + * + * Left unchecked the failure is SILENT and expensive: routing succeeds, the + * request and inbox rows are written, and the approver then opens a task she + * cannot open — the wall hides the record. Filtering here converts that into + * the empty-slate path the node already has a policy for + * (`onEmptyApprovers`), and logs exactly who was dropped and why, so the fix + * ("grant the membership" or "retarget") is legible without a debugger. + */ +export async function filterApproversWhoCanRead( + deps: ApproverOrgScopeDeps, + userIds: string[], + requestOrgId: string | null | undefined, + context: { approverType: string; value?: string; directoryOrgId?: string | null }, +): Promise { + const requestOrg = requestOrgId ? String(requestOrgId) : ''; + if (!requestOrg || userIds.length === 0) return userIds; + + let members: any[] = []; + try { + members = await deps.engine.find('sys_member', { + filter: { organization_id: requestOrg, user_id: { $in: userIds } }, + fields: ['user_id'], + limit: 10000, + context: SYSTEM_CTX, + } as any); + } catch { + // Membership unreadable — do NOT drop everyone on an infrastructure + // hiccup. Routing to someone who may not see the request is recoverable + // (an admin can grant the membership); silently emptying a live approval + // slate is not. + return userIds; + } + + const canRead = new Set( + (members ?? []).map((m: any) => String(m?.user_id ?? '')).filter(Boolean), + ); + const dropped = userIds.filter((u) => !canRead.has(u)); + if (dropped.length === 0) return userIds; + + deps.logger?.warn?.( + `[approvals] ADR-0105 D9: ${dropped.length} cross-organization approver(s) dropped — ` + + `they hold '${context.value ?? context.approverType}' in organization ` + + `'${context.directoryOrgId}' but no membership in the request's organization ` + + `'${requestOrg}', so the D2 union wall would hide the request from them. ` + + `Grant those users a membership in the request's organization, or retarget the approver.`, + { dropped, requestOrganizationId: requestOrg, directoryOrganizationId: context.directoryOrgId }, + ); + return userIds.filter((u) => canRead.has(u)); +} diff --git a/packages/spec/api-surface.json b/packages/spec/api-surface.json index 9b52d9d77d..c1be06cc53 100644 --- a/packages/spec/api-surface.json +++ b/packages/spec/api-surface.json @@ -1985,6 +1985,8 @@ "APPROVAL_BRANCH_LABELS (const)", "APPROVAL_NODE_TYPE (const)", "APPROVER_EXPRESSION_ROOTS (const)", + "APPROVER_ORG_SCOPED (const)", + "APPROVER_ORG_SYMBOLS (const)", "APPROVER_VALUE_BINDINGS (const)", "APPROVER_VALUE_SOURCES (const)", "ActionCategory (type)", @@ -2003,6 +2005,7 @@ "ApprovalNodeApproverSchema (const)", "ApprovalNodeConfig (type)", "ApprovalNodeConfigSchema (const)", + "ApproverOrgSymbol (type)", "ApproverType (const)", "ApproverValueBinding (type)", "AuthField (type)", @@ -2176,6 +2179,7 @@ "WebhookSchema (const)", "WebhookTriggerType (const)", "analyzeRegion (function)", + "approverTypeIsOrgScoped (function)", "canonicalApproverType (function)", "defineActionDescriptor (function)", "defineFlow (function)", diff --git a/packages/spec/src/automation/approval.zod.ts b/packages/spec/src/automation/approval.zod.ts index 7008ef17f5..9e8730a0d0 100644 --- a/packages/spec/src/automation/approval.zod.ts +++ b/packages/spec/src/automation/approval.zod.ts @@ -204,6 +204,67 @@ export const APPROVER_VALUE_SOURCES = Object.fromEntries( | { source: 'auto' | 'trigger-field' | 'expression' | 'unsupported' } >; +// ========================================================================== +// [ADR-0105 D9] Cross-organization approver targeting +// ========================================================================== + +/** + * Symbolic organization references usable as an approver's `organization` + * (ADR-0105 D9). + * + * Flow metadata is PORTABLE — the same flow deploys to dev, staging and every + * customer environment — while an organization id is DATA, minted per + * deployment. A literal id in a flow is therefore unportable by construction, + * and an AI author cannot know one at authoring time. These symbols express + * the two common intents against the D6 grouping tree instead, so the usual + * cases need no deployment knowledge at all: + * + * - `$root` — the group organization: walk `parent_organization_id` up from + * the request's organization to the top. "Escalate to group." + * - `$parent` — exactly one level up. Division-level sign-off in a three-tier + * group (group → division → plant). + * + * A slug (any other value) names a specific organization and covers the shapes + * the symbols cannot: a SIBLING org, e.g. a shared-services centre that + * approves payables for every plant. That is also why D9's legality rule is + * "shares a root", not "is an ancestor". + */ +export const APPROVER_ORG_SYMBOLS = ['$root', '$parent'] as const; +export type ApproverOrgSymbol = (typeof APPROVER_ORG_SYMBOLS)[number]; + +/** + * Whether an approver type resolves people through an ORGANIZATION-SCOPED + * directory — i.e. whether `organization` (ADR-0105 D9) means anything for it. + * + * `user` / `field` / `manager` name a person directly or derive one from the + * record, so they are org-agnostic: an `organization` on them is not a narrower + * routing, it is a misunderstanding, and the lint says so rather than silently + * ignoring it. `team` is org-agnostic too — `sys_team_member` carries no + * organization column and the engine never scoped it (unlike position / + * membership-tier / department, which all do). + * + * `satisfies` keeps this exhaustive for the same reason + * {@link APPROVER_VALUE_BINDINGS} is: a new {@link ApproverType} must state + * whether cross-org targeting applies to it, or this file fails to compile. + */ +export const APPROVER_ORG_SCOPED = { + user: false, + org_membership_level: true, + role: true, + position: true, + team: false, + department: true, + manager: false, + field: false, + queue: false, + expression: true, +} as const satisfies Record, boolean>; + +/** Does `organization` (ADR-0105 D9) apply to this approver type? */ +export function approverTypeIsOrgScoped(type: string): boolean { + return (APPROVER_ORG_SCOPED as Record)[canonicalApproverType(type)] ?? false; +} + // ========================================================================== // Approval as a Flow Node (ADR-0019, canonical) // ========================================================================== @@ -337,6 +398,41 @@ export const ApprovalNodeApproverSchema = lazySchema(() => z.object({ * (keyed by position), so a plain per-approver list still behaves sensibly. */ group: z.string().optional().describe('Group label for per_group sign-off (e.g. "legal", "finance")'), + /** + * [ADR-0105 D9] Which organization's directory this approver is resolved + * AGAINST. Default (omitted): the request's own organization — today's + * behaviour, unchanged. + * + * This separates two things one organization id used to do at once: WHERE + * THE REQUEST LIVES and WHERE ITS APPROVERS ARE LOOKED UP. A group CFO holds + * her `cfo` position in the GROUP organization while the purchase order + * lives in the PLANT organization; without this field + * `expandPositionUsers('cfo', )` finds nobody and the slot routes + * into `onEmptyApprovers`. + * + * Declared per APPROVER, not per node, so one node can require a plant + * manager AND a group CFO in parallel (`behavior: 'per_group'`). A node-level + * default is a strict special case of this and can be added later as sugar; + * the reverse is not true. + * + * Values: `$root` / `$parent` (see {@link APPROVER_ORG_SYMBOLS}) or an + * organization SLUG. Slugs are used rather than ids because flow metadata is + * portable across environments and ids are not. + * + * Bounded, not free: the target must share a `parent_organization_id` root + * with the request's organization (ADR-0105 D6 grouping metadata), so this is + * a route WITHIN one group — never a channel to an unrelated tenant. Applies + * only to org-scoped approver types ({@link APPROVER_ORG_SCOPED}) and only + * under the `group` tenancy posture; elsewhere the runtime refuses rather + * than silently ignoring, so a posture migration cannot quietly reroute + * approvals. + */ + organization: z.string().optional().meta({ + description: + '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.', + xRef: { kind: 'organization', symbols: [...APPROVER_ORG_SYMBOLS] }, + }), })); export type ApprovalNodeApprover = z.infer;