diff --git a/.changeset/approver-value-sources-and-dead-slot-warning.md b/.changeset/approver-value-sources-and-dead-slot-warning.md new file mode 100644 index 0000000000..920f2a061f --- /dev/null +++ b/.changeset/approver-value-sources-and-dead-slot-warning.md @@ -0,0 +1,53 @@ +--- +"@objectstack/spec": minor +"@objectstack/plugin-approvals": patch +--- + +feat(spec)+fix(approvals): publish approver value data sources, order the type enum for authors, stop silent dead approver slots (#3508 / #3807 follow-ups) + +Four follow-ups from browser-verifying the #3508 approver work end to end. + +**`APPROVER_VALUE_SOURCES` — the designer stops guessing where candidates live.** +`xRef.map` only ever named a picker KIND (`'team'`), never where that picker's +rows come from, so the designer carried its own copy of the data contract — and +the first copy was wrong: every directory kind was wired to `GET +/api/v1/meta/:type`, the metadata REGISTRY, which does not hold `sys_user` / +`sys_team` / `sys_business_unit` / `sys_position` rows. Candidates came back +empty and the control degraded to free text (#3508). The binding is now +projected onto the published JSON schema as `xRef.sources` — `{ source: 'data', +object, valueField }` for the record-backed kinds, the closed enum inline for +`org_membership_level` — derived from `APPROVER_VALUE_BINDINGS` so the two +cannot drift, and inheriting its `satisfies` exhaustiveness (a new +`ApproverType` member that declares no source is a compile error). Presentation +— which field to show, whether to open a people-picker, what subtitle to use — +stays a renderer decision. + +**`ApproverType` declaration order is now the authoring recommendation.** +objectui#2834 argued for leading with indirect bindings and shipped that order +in its own options array — which the Studio inspector never reads: it derives +the picker from this enum via the published schema, so `user` still came first. +The intent only takes effect if the enum carries it, so the enum now reads +`manager, position, department, team, field, expression, org_membership_level, +user` (deprecated `role` / `queue` still parse and stay out of every picker via +`xEnumDeprecated`). Binding one specific person is the least portable choice an +author can make — it breaks when the flow moves to another environment (that id +does not exist there) and again when that person leaves. + +**A graph approver that expands to nobody no longer does it in silence.** +`queue` already warned (#3508); every OTHER graph type — `team`, `department`, +`position`, `org_membership_level`, `manager` — fell back to the same +unactionable `type:value` literal without a word. That silence is what let +#3807 hide for as long as it did: the request opened with an empty slate and +the first symptom was a permanently stuck approval (#3424). The fallback stays +(15.x slots and substring fixtures depend on it); it now logs the type, value +and organization that produced it. `user` / `field` stay quiet — they take the +id they were given and never had an "expanded to nobody" state. + +**`plugin-sharing`'s identical org scope is pinned by tests.** +`BusinessUnitGraphService.orgScope` has the same strict `organization_id` +equality #3807 fixed in approvals. It is unreachable today — every materialized +`sys_sharing_rule` carries `organization_id = null`, so the filter is skipped — +and widening an authorization path on a defect that cannot currently fire is +not a change to make blind. New tests lock both the reachable paths and the +divergence itself, so if sharing ever adopts the null-org=env-wide reading it +is a deliberate edit to a named test rather than a silent behaviour change. diff --git a/content/docs/references/automation/approval.mdx b/content/docs/references/automation/approval.mdx index c6436ee165..7521b9b912 100644 --- a/content/docs/references/automation/approval.mdx +++ b/content/docs/references/automation/approval.mdx @@ -7,6 +7,12 @@ description: Approval protocol schemas Approval Step Approver Type +Declaration order is author-facing: designers derive their picker from this + +enum, and it leads with the portable indirect bindings (`manager`, + +`position`, `department`, `team`) — a literal `user` id comes last. + **Source:** `packages/spec/src/automation/approval.zod.ts` @@ -54,7 +60,7 @@ const result = ApprovalDecision.parse(data); | Property | Type | Required | Description | | :--- | :--- | :--- | :--- | -| **type** | `Enum<'user' \| 'org_membership_level' \| 'role' \| 'position' \| 'team' \| 'department' \| 'manager' \| 'field' \| 'queue' \| 'expression'>` | ✅ | | +| **type** | `Enum<'manager' \| 'position' \| 'department' \| 'team' \| 'field' \| 'expression' \| 'org_membership_level' \| 'role' \| 'user' \| 'queue'>` | ✅ | | | **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") | @@ -68,7 +74,7 @@ const result = ApprovalDecision.parse(data); | Property | Type | Required | Description | | :--- | :--- | :--- | :--- | -| **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 | +| **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 | @@ -85,16 +91,16 @@ const result = ApprovalDecision.parse(data); ### Allowed Values -* `user` -* `org_membership_level` -* `role` +* `manager` * `position` -* `team` * `department` -* `manager` +* `team` * `field` -* `queue` * `expression` +* `org_membership_level` +* `role` +* `user` +* `queue` --- diff --git a/packages/plugins/plugin-approvals/src/approval-service.test.ts b/packages/plugins/plugin-approvals/src/approval-service.test.ts index 0523b0a39e..a0dc124d76 100644 --- a/packages/plugins/plugin-approvals/src/approval-service.test.ts +++ b/packages/plugins/plugin-approvals/src/approval-service.test.ts @@ -2331,6 +2331,66 @@ describe('ApprovalService — queue approver is unresolved (#3508)', () => { }); }); +// #3807 follow-up: `queue` was not the only way to end up with a slot nobody +// can act on — every GRAPH approver type falls back to the same literal when +// its lookup finds no one, and that fallback used to happen in total silence. +// A stuck approval was the first symptom; the log said nothing. The literal +// stays (15.x slots and substring fixtures depend on it) — it just announces +// itself now. +describe('ApprovalService — a graph approver that expands to nobody warns (#3807)', () => { + const svcWithWarnings = (engine: any) => { + const warnings: any[] = []; + let n = 0; + const svc = new ApprovalService({ + engine, + clock: { now: () => new Date(1757000000000 + (n++) * 1000) }, + logger: { warn: (msg: any, meta: any) => warnings.push([msg, meta]) }, + }); + return { svc, warnings }; + }; + + const approverInput = (type: string, value: string) => ({ + ...openInput([]), + config: { approvers: [{ type, value }], behavior: 'first_response' as const, lockRecord: false }, + }); + + it.each([ + ['team', 'team_gone'], + ['department', 'bu_gone'], + ['position', 'nobody_holds_this'], + ['org_membership_level', 'member'], + ])('%s: the dead literal is logged with its type, value and org', async (type, value) => { + const engine = makeFakeEngine(); + const { svc, warnings } = svcWithWarnings(engine); + const req = await svc.openNodeRequest(approverInput(type, value), CTX); + + expect(req.pending_approvers).toEqual([`${type}:${value}`]); + const hit = warnings.find(([msg]) => String(msg).includes('expanded to nobody')); + expect(hit, `no warning for ${type}`).toBeTruthy(); + expect(String(hit[0])).toContain('#3807'); + expect(hit[1]).toMatchObject({ type, value, organizationId: 't1' }); + }); + + it('stays quiet when the graph DOES resolve someone', async () => { + const engine = makeFakeEngine(); + engine._tables['sys_team_member'] = [{ id: 'tm1', team_id: 'team_ok', user_id: 'u5' }]; + const { svc, warnings } = svcWithWarnings(engine); + const req = await svc.openNodeRequest(approverInput('team', 'team_ok'), CTX); + + expect(req.pending_approvers).toEqual(['u5']); + expect(warnings.filter(([msg]) => String(msg).includes('expanded to nobody'))).toEqual([]); + }); + + it('stays quiet for `user` — a literal id was never a lookup that could come back empty', async () => { + const engine = makeFakeEngine(); + const { svc, warnings } = svcWithWarnings(engine); + const req = await svc.openNodeRequest(approverInput('user', 'u_unknown'), CTX); + + expect(req.pending_approvers).toEqual(['u_unknown']); + expect(warnings.filter(([msg]) => String(msg).includes('expanded to nobody'))).toEqual([]); + }); +}); + // ── File-access delegate (ADR-0104 D3 wave 2) ──────────────────────── // // A decision attachment is OWNED by its `sys_approval_action` row, so the diff --git a/packages/plugins/plugin-approvals/src/approval-service.ts b/packages/plugins/plugin-approvals/src/approval-service.ts index 1ca3ebde7d..dc591e072d 100644 --- a/packages/plugins/plugin-approvals/src/approval-service.ts +++ b/packages/plugins/plugin-approvals/src/approval-service.ts @@ -177,6 +177,21 @@ function actingUserId(context: SharingExecutionContext | undefined): string | nu */ const OOO_MAX_CHAIN = 8; +/** + * Approver types resolved by QUERYING a graph rather than by taking `value` + * literally (#3807). Each can legitimately come back empty — an unstaffed + * position, an emptied team, a mis-pointed unit — and the caller then falls + * back to a `type:value` literal that no user can act on. They are listed here + * so that dead end gets one warning instead of passing in silence. + * + * `user` / `field` are deliberately absent: they resolve to the id they were + * given without a lookup, so there is no "expanded to nobody" state to report. + * `business_unit` / `bu` are the accepted dialects of `department`. + */ +const GRAPH_APPROVER_TYPES: ReadonlySet = new Set([ + 'team', 'department', 'business_unit', 'bu', 'position', 'org_membership_level', 'manager', +]); + /** One OOO delegation hop applied while resolving an approver (#1322 M1/M4). */ interface OooSubstitution { /** The approver who was skipped (out of office). */ @@ -757,6 +772,21 @@ export class ApprovalService implements IApprovalService { `[approvals] approver type 'queue' is not implemented — the slot resolves to nobody (#3508)`, { value: a.value }, ); + } else if (GRAPH_APPROVER_TYPES.has(type)) { + // #3807 follow-up — every OTHER way to land here is a graph type whose + // lookup produced nobody, and the literal below is a slot no user can + // ever act on. That silence is what let #3807 hide: a `department` + // approver pointing at a seeded (env-wide) unit resolved to + // `department:` on every request, the request opened with an empty + // slate, and nothing in the logs said so — the first symptom was a + // permanently stuck approval (#3424). The fallback itself stays (a + // literal keeps 15.x slots and substring fixtures working); it just + // stops being invisible. + this.logger?.warn?.( + `[approvals] approver '${type}:${a.value}' expanded to nobody — the slot routes to no one ` + + `and the request cannot advance until someone is added or the approver is re-pointed (#3807)`, + { type, value: a.value, organizationId: organizationId ?? null }, + ); } return [`${a.type}:${a.value}`]; } diff --git a/packages/plugins/plugin-sharing/src/business-unit-graph.test.ts b/packages/plugins/plugin-sharing/src/business-unit-graph.test.ts new file mode 100644 index 0000000000..d61546128c --- /dev/null +++ b/packages/plugins/plugin-sharing/src/business-unit-graph.test.ts @@ -0,0 +1,162 @@ +// Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license. + +/** + * BusinessUnitGraphService — org scoping of the unit tree. + * + * These pin the ORG-SCOPE behaviour specifically, because it is the exact + * shape that broke approvals in #3807: `orgScope()` AND-composes a strict + * `organization_id = ` equality, so a unit written with no + * organization at all (a seeded / file-layer / bootstrap row — a seed cannot + * know the org id the runtime mints at boot) matches nothing, the seed check + * fails, and the expansion returns zero members. In approvals that produced a + * dead `department:` approver slot; here it would produce a sharing rule + * that silently grants nobody. + * + * It is NOT reachable today: every materialized `sys_sharing_rule` row carries + * `organization_id = null` (verified on a live showcase stack), so + * `expandRecipient` passes `null` and `orgScope` is skipped entirely. The + * moment rules start carrying an org — a multi-tenant deployment — a BU + * subtree rule against a seeded unit stops granting, and the symptom is + * "the right people cannot see the record", which is far quieter than a stuck + * approval. + * + * So this file locks BOTH sides down: + * - the reachable paths (null-org rule) keep working, and + * - the divergence from approvals is written down as an executable fact + * rather than a comment, so flipping it is a deliberate edit to a named + * test and never a silent behaviour change. + * + * If the platform decides null-org means "env-wide, visible to every org" for + * sharing too — the way `plugin-approvals` and `sys_metadata` already read it — + * the test named `[divergence]` below is the one to flip, and `orgScope` grows + * the same `$or: [{ organization_id }, { organization_id: null }]` predicate. + */ + +import { describe, it, expect } from 'vitest'; +import { BusinessUnitGraphService } from './business-unit-graph.js'; + +interface UnitRow { + id: string; + parent_business_unit_id?: string | null; + organization_id?: string | null; + active?: boolean; +} +interface MemberRow { business_unit_id: string; user_id: string } + +/** + * Minimal engine over `sys_business_unit` + `sys_business_unit_member`. + * Mirrors the real filter surface the service uses: plain equality, `$ne`, + * `$in`, and `$or` (so a widened predicate would actually be exercised here + * rather than silently ignored by the stub). + */ +function makeEngine(units: UnitRow[], members: MemberRow[]) { + const matches = (row: any, 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 && typeof v === 'object' && '$ne' in (v as any)) { + if (rv === (v as any).$ne) return false; + continue; + } + if (v && typeof v === 'object' && '$in' in (v as any)) { + if (!(v as any).$in.includes(rv)) return false; + continue; + } + // `organization_id: null` must also match a row that simply omits the + // column — that is what a NULL column reads back as. + if (v === null) { + if (rv != null) return false; + continue; + } + if (rv !== v) return false; + } + return true; + }; + + return { + async find(object: string, options: any) { + const filter = options?.where ?? options?.filter ?? {}; + if (object === 'sys_business_unit') return units.filter((u) => matches(u, filter)); + if (object === 'sys_business_unit_member') return members.filter((m) => matches(m, filter)); + return []; + }, + } as any; +} + +/** A seeded org tree: rows written before any organization existed. */ +const SEEDED_UNITS: UnitRow[] = [ + { id: 'bu_root', organization_id: null, active: true }, + { id: 'bu_child', parent_business_unit_id: 'bu_root', organization_id: null, active: true }, +]; +const SEEDED_MEMBERS: MemberRow[] = [ + { business_unit_id: 'bu_root', user_id: 'u_root' }, + { business_unit_id: 'bu_child', user_id: 'u_child' }, +]; + +describe('BusinessUnitGraphService — subtree expansion', () => { + it('expands the unit and every descendant', async () => { + const g = new BusinessUnitGraphService({ engine: makeEngine(SEEDED_UNITS, SEEDED_MEMBERS) }); + expect((await g.expandUsers('bu_root')).sort()).toEqual(['u_child', 'u_root']); + }); + + it('an inactive unit contributes nobody and stops the descent', async () => { + const units: UnitRow[] = [ + { id: 'bu_root', organization_id: null, active: false }, + { id: 'bu_child', parent_business_unit_id: 'bu_root', organization_id: null, active: true }, + ]; + const g = new BusinessUnitGraphService({ engine: makeEngine(units, SEEDED_MEMBERS) }); + expect(await g.expandUsers('bu_root')).toEqual([]); + }); +}); + +describe('BusinessUnitGraphService — org scoping (#3807)', () => { + it('an org-less rule (today’s materialized shape) expands seeded units fine', async () => { + // `expandRecipient` passes `rule.organization_id ?? null`, and every + // materialized sharing rule carries null — so `orgScope` adds nothing and + // the seeded tree resolves. This is the only path that runs in practice. + const g = new BusinessUnitGraphService({ + engine: makeEngine(SEEDED_UNITS, SEEDED_MEMBERS), + organizationId: null, + }); + expect((await g.expandUsers('bu_root')).sort()).toEqual(['u_child', 'u_root']); + }); + + it('an org-scoped rule expands units belonging to that org', async () => { + const units: UnitRow[] = [ + { id: 'bu_root', organization_id: 'org_a', active: true }, + { id: 'bu_child', parent_business_unit_id: 'bu_root', organization_id: 'org_a', active: true }, + ]; + const g = new BusinessUnitGraphService({ + engine: makeEngine(units, SEEDED_MEMBERS), + organizationId: 'org_a', + }); + expect((await g.expandUsers('bu_root')).sort()).toEqual(['u_child', 'u_root']); + }); + + it('an org-scoped rule never reaches another org’s unit', async () => { + const units: UnitRow[] = [{ id: 'bu_root', organization_id: 'org_b', active: true }]; + const g = new BusinessUnitGraphService({ + engine: makeEngine(units, SEEDED_MEMBERS), + organizationId: 'org_a', + }); + expect(await g.expandUsers('bu_root')).toEqual([]); + }); + + it('[divergence] an org-scoped rule does NOT see an env-wide (null-org) unit — approvals does (#3807)', async () => { + // Same inputs that #3807 fixed on the approvals side. Sharing still reads + // a null-org unit as "belongs to no org, therefore not mine" and grants + // nobody. Unreachable today (rules are null-org), deliberate until the + // platform rules on null-org semantics for AUTHORIZATION paths — widening + // who can SEE a record is not a change to make on a defect that cannot + // currently fire. + const g = new BusinessUnitGraphService({ + engine: makeEngine(SEEDED_UNITS, SEEDED_MEMBERS), + organizationId: 'org_a', + }); + expect(await g.expandUsers('bu_root')).toEqual([]); + }); +}); diff --git a/packages/spec/api-surface.json b/packages/spec/api-surface.json index acfb19e2e9..377492022c 100644 --- a/packages/spec/api-surface.json +++ b/packages/spec/api-surface.json @@ -1982,6 +1982,7 @@ "APPROVAL_NODE_TYPE (const)", "APPROVER_EXPRESSION_ROOTS (const)", "APPROVER_VALUE_BINDINGS (const)", + "APPROVER_VALUE_SOURCES (const)", "ActionCategory (type)", "ActionCategorySchema (const)", "ActionDescriptor (type)", diff --git a/packages/spec/src/automation/approval.test.ts b/packages/spec/src/automation/approval.test.ts index d6111ea496..618b0edce3 100644 --- a/packages/spec/src/automation/approval.test.ts +++ b/packages/spec/src/automation/approval.test.ts @@ -4,6 +4,7 @@ import { DEPRECATED_APPROVER_TYPES, NON_AUTHORABLE_APPROVER_TYPES, APPROVER_VALUE_BINDINGS, + APPROVER_VALUE_SOURCES, ORG_MEMBERSHIP_LEVELS, canonicalApproverType, APPROVAL_NODE_TYPE, @@ -116,6 +117,66 @@ describe('APPROVER_VALUE_BINDINGS (#3508)', () => { }); }); +// #3508 follow-up: `xRef.map` names a picker KIND but never said where that +// picker's candidates live — which is how the designer came to query the +// metadata registry for data records. The data contract now ships on the wire. +describe('APPROVER_VALUE_SOURCES (#3508 follow-up)', () => { + it('covers every ApproverType member, exactly like the bindings it projects', () => { + for (const t of ApproverType.options) { + expect(APPROVER_VALUE_SOURCES[t], `no source published for '${t}'`).toBeDefined(); + expect(APPROVER_VALUE_SOURCES[t].source).toBe( + APPROVER_VALUE_BINDINGS[t].source === 'record' ? 'data' : APPROVER_VALUE_BINDINGS[t].source, + ); + } + }); + + it('routes the directory kinds to the DATA API, naming the object and the committed column', () => { + // `data`, not `meta`: these are rows in system objects, and the metadata + // registry (`GET /api/v1/meta/:type`) cannot list them (#3508). + expect(APPROVER_VALUE_SOURCES.user).toEqual({ source: 'data', object: 'sys_user', valueField: 'id' }); + expect(APPROVER_VALUE_SOURCES.team).toEqual({ source: 'data', object: 'sys_team', valueField: 'id' }); + expect(APPROVER_VALUE_SOURCES.department).toEqual({ source: 'data', object: 'sys_business_unit', valueField: 'id' }); + // The one kind that commits something other than the primary key. + expect(APPROVER_VALUE_SOURCES.position).toEqual({ source: 'data', object: 'sys_position', valueField: 'name' }); + }); + + it('keeps the non-record kinds off the lookup path and carries the closed enum inline', () => { + expect(APPROVER_VALUE_SOURCES.org_membership_level).toEqual({ source: 'enum', values: [...ORG_MEMBERSHIP_LEVELS] }); + expect(APPROVER_VALUE_SOURCES.role).toEqual(APPROVER_VALUE_SOURCES.org_membership_level); + expect(APPROVER_VALUE_SOURCES.manager).toEqual({ source: 'auto' }); + expect(APPROVER_VALUE_SOURCES.field).toEqual({ source: 'trigger-field' }); + expect(APPROVER_VALUE_SOURCES.expression).toEqual({ source: 'expression' }); + expect(APPROVER_VALUE_SOURCES.queue).toEqual({ source: 'unsupported' }); + }); + + it('ships on the published JSON schema, where the designer reads it', () => { + const schema = getApprovalNodeConfigJsonSchema() as any; + const sources = schema?.properties?.approvers?.items?.properties?.value?.xRef?.sources; + expect(sources).toBeDefined(); + expect(sources.department).toEqual({ source: 'data', object: 'sys_business_unit', valueField: 'id' }); + expect(sources.position).toEqual({ source: 'data', object: 'sys_position', valueField: 'name' }); + }); +}); + +// objectui#2834 argued for leading with indirect bindings, but shipped the +// order in its own options array — which the Studio inspector never reads (it +// derives the picker from this enum via the published JSON schema). The intent +// only takes effect if the enum itself carries it, so pin it here. +describe('ApproverType declaration order is the authoring recommendation', () => { + const authorable = ApproverType.options.filter((t) => !NON_AUTHORABLE_APPROVER_TYPES.includes(t)); + + it('leads with indirect bindings and puts the literal `user` last', () => { + expect(authorable).toEqual([ + 'manager', 'position', 'department', 'team', 'field', 'expression', 'org_membership_level', 'user', + ]); + }); + + it('never offers a non-authorable spelling, wherever it sits in the enum', () => { + expect(authorable).not.toContain('role'); + expect(authorable).not.toContain('queue'); + }); +}); + describe('Approval node constants (ADR-0019)', () => { it('exposes the canonical node type and decision branch labels', () => { expect(APPROVAL_NODE_TYPE).toBe('approval'); diff --git a/packages/spec/src/automation/approval.zod.ts b/packages/spec/src/automation/approval.zod.ts index 09a876de3c..7008ef17f5 100644 --- a/packages/spec/src/automation/approval.zod.ts +++ b/packages/spec/src/automation/approval.zod.ts @@ -3,11 +3,46 @@ import { z } from 'zod'; import { lazySchema } from '../shared/lazy-schema'; +// Why the members sit in THIS order (the generated reference renders the JSDoc +// below; this rationale stays in source): +// +// A designer that derives its approver-type picker from this enum — the Studio +// flow inspector does, via the published JSON schema minus `xEnumDeprecated` — +// offers the members in exactly this order. So the order is a recommendation +// the platform makes to every author, not an accident of when each member was +// added, and it belongs here rather than in any one renderer's options array +// (objectui#2834 put it there, where the inspector never read it). +// +// Indirect bindings therefore lead and the literal `user` binding comes last. +// Naming one specific person is the least portable choice an author can make: +// it breaks when the flow is deployed to another environment (that id does not +// exist there), and again when that person changes team or leaves — silently +// routing approvals to someone who should no longer see them. `manager` / +// `position` / `department` / `team` survive both. /** * Approval Step Approver Type + * + * Declaration order is author-facing: designers derive their picker from this + * enum, and it leads with the portable indirect bindings (`manager`, + * `position`, `department`, `team`) — a literal `user` id comes last. */ export const ApproverType = z.enum([ - 'user', // Specific user(s) + 'manager', // Submitter's manager (sys_user.manager_id) + 'position', // Holders of a position (sys_user_position, ADR-0090 D3) + 'department', // Members of a department + all descendant departments (sys_business_unit) + 'team', // Members of a flat collaboration team (sys_team) + 'field', // User ID defined in a record field + /** + * #3447 P2: a CEL expression resolved AT NODE ENTRY against three explicit + * roots — `current.*` (the record's live state), `trigger.*` (the submit-time + * snapshot) and `vars.*` (flow variables, incl. upstream node outputs). The + * result (a user-id string, CSV, or string array — or intermediate ids + * re-expanded per `resolveAs`) becomes the approver slate. `record` and bare + * field names are deliberately NOT available: `record` means "the record at + * event time" everywhere else on the platform (flow conditions, hooks), and + * reusing it here would silently alias one of the two times. + */ + 'expression', // The better-auth ORG-MEMBERSHIP TIER (sys_member.role: owner / admin / // member), spelled with the projection name ADR-0057 D7 mandates and // ADR-0090 D3 assumes ("relabelled `org_membership_level` … its UI label is @@ -19,13 +54,11 @@ export const ApproverType = z.enum([ // surfaces; its exception covers better-auth's own `sys_member.role` column, // NOT this enum (which is ours). Accepted for one deprecation window: the // runtime resolves it identically and warns, `os lint` prescribes the - // rewrite. Removed in the next major. + // rewrite. Removed in the next major. Kept adjacent to the spelling that + // replaced it; `xEnumDeprecated` keeps it out of every picker regardless of + // where it sits. 'role', - 'position', // Holders of a position (sys_user_position, ADR-0090 D3) - 'team', // Members of a flat collaboration team (sys_team) - 'department', // Members of a department + all descendant departments (sys_business_unit) - 'manager', // Submitter's manager (sys_user.manager_id) - 'field', // User ID defined in a record field + 'user', // A specific user id — least portable; see the note above // @deprecated #3508 — declared-but-unenforced: `resolveApproverSpec` in // `plugin-approvals` has no queue branch, so a queue approver resolves to // nobody (the dead `queue:` fallback literal). The sharing engine @@ -34,17 +67,6 @@ export const ApproverType = z.enum([ // (see NON_AUTHORABLE_APPROVER_TYPES). Re-admit only together with a real // ownership-queue implementation (queue entity + membership + claim). 'queue', - /** - * #3447 P2: a CEL expression resolved AT NODE ENTRY against three explicit - * roots — `current.*` (the record's live state), `trigger.*` (the submit-time - * snapshot) and `vars.*` (flow variables, incl. upstream node outputs). The - * result (a user-id string, CSV, or string array — or intermediate ids - * re-expanded per `resolveAs`) becomes the approver slate. `record` and bare - * field names are deliberately NOT available: `record` means "the record at - * event time" everywhere else on the platform (flow conditions, hooks), and - * reusing it here would silently alias one of the two times. - */ - 'expression' ]); /** @@ -141,6 +163,47 @@ export const APPROVER_VALUE_BINDINGS = { expression: { source: 'expression', roots: APPROVER_EXPRESSION_ROOTS }, } as const satisfies Record, ApproverValueBinding>; +/** + * {@link APPROVER_VALUE_BINDINGS}, projected onto the wire for the designer + * (#3508 follow-up). + * + * `xRef.map` only ever said which PICKER KIND to render — a name like `'team'` + * — and never where that picker's candidates come from. So the designer had to + * carry its own copy of the data contract, and the first copy was wrong: every + * directory kind was wired to `GET /api/v1/meta/:type`, the metadata registry, + * which does not hold `sys_user` / `sys_team` / `sys_business_unit` / + * `sys_position` ROWS. Candidates came back empty and the control degraded to a + * free-text box (#3508). + * + * Publishing the binding closes that gap at the source: a renderer reads which + * object to query and which column to commit off the schema instead of + * re-deriving it, and a new {@link ApproverType} member cannot leave a stale + * mirror behind — `satisfies` above already makes an undeclared member a + * compile error, and this projection inherits that guarantee. + * + * Presentation stays with the renderer: which field to SHOW, whether to open a + * people-picker, what subtitle to put under a row are objectui's calls. This + * carries only the data contract — where the candidates live and what the + * committed value is. + */ +export const APPROVER_VALUE_SOURCES = Object.fromEntries( + Object.entries(APPROVER_VALUE_BINDINGS).map(([type, binding]) => [ + type, + binding.source === 'record' + // `data` names the DATA API, in contrast to the metadata registry the + // designer used to query — the whole point of the annotation. + ? { source: 'data', object: binding.object, valueField: binding.valueField } + : binding.source === 'enum' + ? { source: 'enum', values: [...binding.values] } + : { source: binding.source }, + ]), +) as Record< + z.infer, + | { source: 'data'; object: string; valueField: string } + | { source: 'enum'; values: string[] } + | { source: 'auto' | 'trigger-field' | 'expression' | 'unsupported' } +>; + // ========================================================================== // Approval as a Flow Node (ADR-0019, canonical) // ========================================================================== @@ -247,6 +310,11 @@ export const ApprovalNodeApproverSchema = lazySchema(() => z.object({ field: 'object-field', queue: 'queue', }, + // Where each kind's candidates actually live, and what the picker + // commits — see {@link APPROVER_VALUE_SOURCES}. `map` alone named a + // picker but never its data source, which is how the designer ended up + // querying the metadata registry for data records (#3508). + sources: APPROVER_VALUE_SOURCES, }, }), /**