diff --git a/.changeset/approver-type-org-membership-level.md b/.changeset/approver-type-org-membership-level.md new file mode 100644 index 0000000000..d4e1899b0f --- /dev/null +++ b/.changeset/approver-type-org-membership-level.md @@ -0,0 +1,44 @@ +--- +"@objectstack/spec": minor +"@objectstack/plugin-approvals": minor +"@objectstack/lint": minor +--- + +feat(approvals): rename the `role` approver type to `org_membership_level` (#3133) + +`ApproverType.role` was the last platform surface projecting the reserved word +"role" (ADR-0090 D3). It is not covered by D3's better-auth exception: that +exception protects better-auth's own `sys_member.role` **column**, which we do +not own — `ApproverType` is our own enum, an authoring surface, and D3 mandates +that the projection of that concept is spelled `org_membership_level` and +labelled "organization membership", **never "role"**. + +The sentence licensing the leak was also false: ADR-0090 D3 claims +`sys_member.role` is "already relabelled `org_membership_level` in the platform +projection", but `org_membership_level` existed nowhere in the codebase and +ADR-0057 D7 lists that relabel under "Deferred (evidence-gated, P4)". The +projection never landed, so the word reached authors. + +The name manufactured a real, silent failure — "hotcrm class": every other +surface renamed to `position` (`sys_role`, `ShareRecipientType.role`, +`ctx.roles[]`), so `{ type: 'role', value: 'sales_manager' }` reads as the +legacy spelling of a position. It resolves against the membership tier, finds +no member row, falls back to an inert `role:sales_manager` literal, and the +request waits forever on an approver that cannot exist. + +- **spec**: `ApproverType` gains `org_membership_level`; `role` stays as a + deprecated alias for one window (a published 15.x flow keeps loading) with + `DEPRECATED_APPROVER_TYPES` + `canonicalApproverType()` as the single source + for the mapping. Removed in the next major. +- **plugin-approvals**: resolves on the canonical type and warns on the + deprecated spelling. The `type:value` fallback literal keeps the **authored** + spelling — stored `sys_approval_approver` rows and `pending_approvers` slots + from 15.x carry `role:`, and rewriting it would orphan them. +- **lint**: `approval-role-not-membership-tier` → `approval-approver-not-membership-tier` + (the rule id carried the reserved word too), plus a new + `approval-approver-type-deprecated`. The two are mutually exclusive: a bad + *value* wins, because prescribing `org_membership_level` for a position name + would be wrong advice — the fix there is `position`. + +Authoring `type: 'role'` keeps working and now says so out loud. Rewrite it as +`org_membership_level`; if the value is an org position, the fix is `position`. diff --git a/content/docs/automation/approvals.mdx b/content/docs/automation/approvals.mdx index 86878fd4a9..121697c5d9 100644 --- a/content/docs/automation/approvals.mdx +++ b/content/docs/automation/approvals.mdx @@ -53,11 +53,14 @@ defineFlow({ ``` -**`position` vs `role`.** `{ type: 'position', value: 'finance_manager' }` routes to the -holders of a position (`sys_user_position`, ADR-0090 D3). The `role` approver type is the -better-auth **org-membership tier** (`sys_member.role`: `owner`/`admin`/`member`) — a position -name authored as `type: 'role'` matches nobody and the request stalls; `os lint` flags this -(`approval-role-not-membership-tier`). +**`position` vs `org_membership_level`.** `{ type: 'position', value: 'finance_manager' }` routes +to the holders of a position (`sys_user_position`, ADR-0090 D3). `org_membership_level` is a +different thing — the better-auth **org-membership tier**, whose only values are +`owner`/`admin`/`member`. A position name authored there matches nobody and the request stalls; +`os lint` flags it (`approval-approver-not-membership-tier`). + +Authored `type: 'role'` on 15.x? That is the deprecated spelling of `org_membership_level` +(ADR-0090 D3): it still resolves, warns at runtime, and is removed in the next major. Approving is itself a gated action — model "may approve" as a capability (`approve_invoice`) the approver's permission set grants, and gate the approve action's `requiredPermissions` on it so the gate is enforced on **both** the UI and the server (ADR-0066 D4). @@ -133,9 +136,11 @@ Only `approvers` is required on the node; everything else has a default (`behavior: 'first_response'`, `lockRecord: true`, `maxRevisions: 3`). Approver entries resolve by kind — `position`, `user`, `field`, `manager`, -`team`, `department`, `queue`, and the membership-tier kind described in the +`team`, `department`, `queue`, and `org_membership_level`, described in the [callout above](#3-the-approval-node), which is the one that silently resolves -to nobody when it's mistaken for a business hierarchy. +to nobody when it's mistaken for a business hierarchy. 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. ### The approver finds it in their queue @@ -148,9 +153,10 @@ curl -b cookies.txt \ "https://your-app.example.com/api/v1/approvals/requests?status=pending&approverId=usr_123" ``` -`approverId` accepts a user id, an email, or a membership-tier reference — and -takes several values (comma-separated or repeated) to cover a person's -identities in one call. +`approverId` accepts a user id, an email, or a `:` approver literal +(`position:finance_manager` — the form an entry falls back to when it resolves +to no users) — and takes several values (comma-separated or repeated) to cover +a person's identities in one call. Other filters: `status`, `object`, `recordId`, `submitterId`, `q`, `limit`, `offset`. diff --git a/content/docs/references/automation/approval.mdx b/content/docs/references/automation/approval.mdx index d462d20272..204eb44a97 100644 --- a/content/docs/references/automation/approval.mdx +++ b/content/docs/references/automation/approval.mdx @@ -54,7 +54,7 @@ const result = ApprovalDecision.parse(data); | Property | Type | Required | Description | | :--- | :--- | :--- | :--- | -| **type** | `Enum<'user' \| 'role' \| 'position' \| 'team' \| 'department' \| 'manager' \| 'field' \| 'queue'>` | ✅ | | +| **type** | `Enum<'user' \| 'org_membership_level' \| 'role' \| 'position' \| 'team' \| 'department' \| 'manager' \| 'field' \| 'queue'>` | ✅ | | | **value** | `string` | optional | User id / membership tier / position / team / department / field / queue — per `type` | @@ -66,7 +66,7 @@ const result = ApprovalDecision.parse(data); | Property | Type | Required | Description | | :--- | :--- | :--- | :--- | -| **approvers** | `{ type: Enum<'user' \| 'role' \| 'position' \| 'team' \| 'department' \| 'manager' \| 'field' \| 'queue'>; value?: string }[]` | ✅ | Allowed approvers for this node | +| **approvers** | `{ type: Enum<'user' \| 'org_membership_level' \| 'role' \| 'position' \| 'team' \| 'department' \| 'manager' \| 'field' \| 'queue'>; value?: string }[]` | ✅ | Allowed approvers for this node | | **behavior** | `Enum<'first_response' \| 'unanimous'>` | ✅ | How to combine multiple approvers | | **lockRecord** | `boolean` | ✅ | Lock the record from editing while pending | | **approvalStatusField** | `string` | optional | Business-object field to mirror request status onto | @@ -81,6 +81,7 @@ const result = ApprovalDecision.parse(data); ### Allowed Values * `user` +* `org_membership_level` * `role` * `position` * `team` diff --git a/packages/lint/src/index.ts b/packages/lint/src/index.ts index 4cab1b79ee..336cd13684 100644 --- a/packages/lint/src/index.ts +++ b/packages/lint/src/index.ts @@ -101,7 +101,8 @@ export type { CapabilityRefFinding, CapabilityRefSeverity } from './validate-cap export { validateApprovalApprovers, - APPROVAL_ROLE_NOT_MEMBERSHIP_TIER, + APPROVAL_APPROVER_NOT_MEMBERSHIP_TIER, + APPROVAL_APPROVER_TYPE_DEPRECATED, APPROVAL_APPROVER_TYPE_UNKNOWN, APPROVAL_ESCALATION_REASSIGN_NO_TARGET, } 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 0de761901f..5956f7f298 100644 --- a/packages/lint/src/validate-approval-approvers.test.ts +++ b/packages/lint/src/validate-approval-approvers.test.ts @@ -3,7 +3,8 @@ import { describe, it, expect } from 'vitest'; import { validateApprovalApprovers, - APPROVAL_ROLE_NOT_MEMBERSHIP_TIER, + APPROVAL_APPROVER_NOT_MEMBERSHIP_TIER, + APPROVAL_APPROVER_TYPE_DEPRECATED, APPROVAL_APPROVER_TYPE_UNKNOWN, APPROVAL_ESCALATION_REASSIGN_NO_TARGET, } from './validate-approval-approvers.js'; @@ -27,28 +28,54 @@ describe('validateApprovalApprovers', () => { expect(validateApprovalApprovers({ flows: [] })).toEqual([]); }); - it('accepts membership tiers for type role (owner/admin/member/guest)', () => { + it('accepts membership tiers for org_membership_level (owner/admin/member/guest)', () => { const findings = validateApprovalApprovers(stackWithApprovers([ - { type: 'role', value: 'admin' }, - { type: 'role', value: 'Owner' }, // case-insensitive - { type: 'role', value: 'member' }, - { type: 'role', value: 'guest' }, + { type: 'org_membership_level', value: 'admin' }, + { type: 'org_membership_level', value: 'Owner' }, // case-insensitive + { type: 'org_membership_level', value: 'member' }, + { type: 'org_membership_level', value: 'guest' }, ])); expect(findings).toEqual([]); }); - it("flags a position name authored as type 'role' (the ADR-0090 D3 hotcrm class)", () => { + it("flags a position name authored as a membership tier (the ADR-0090 D3 hotcrm class)", () => { const findings = validateApprovalApprovers(stackWithApprovers([ - { type: 'role', value: 'sales_manager' }, + { type: 'org_membership_level', value: 'sales_manager' }, ])); expect(findings).toHaveLength(1); - expect(findings[0].rule).toBe(APPROVAL_ROLE_NOT_MEMBERSHIP_TIER); + expect(findings[0].rule).toBe(APPROVAL_APPROVER_NOT_MEMBERSHIP_TIER); expect(findings[0].severity).toBe('warning'); expect(findings[0].where).toContain('expense_approval'); expect(findings[0].path).toBe('flows[0].nodes[1].config.approvers[0].value'); expect(findings[0].hint).toContain("type: 'position'"); }); + // ── the deprecated `role` spelling (ADR-0090 D3, #3133) ────────────────── + + it('flags the deprecated `role` spelling even when its value is a valid tier', () => { + const findings = validateApprovalApprovers(stackWithApprovers([ + { type: 'role', value: 'admin' }, + ])); + expect(findings).toHaveLength(1); + expect(findings[0].rule).toBe(APPROVAL_APPROVER_TYPE_DEPRECATED); + expect(findings[0].path).toBe('flows[0].nodes[1].config.approvers[0].type'); + expect(findings[0].hint).toContain("type: 'org_membership_level'"); + }); + + // The two rules must not both fire: rewriting { type: 'role', value: + // 'sales_manager' } as `org_membership_level` is WRONG advice — the value is + // a position, so `position` is the fix and the deprecation is beside the + // point. Exactly one finding, and it must be the value rule. + it('prefers the value fix over the deprecation notice for a position name', () => { + const findings = validateApprovalApprovers(stackWithApprovers([ + { type: 'role', value: 'sales_manager' }, + ])); + expect(findings).toHaveLength(1); + expect(findings[0].rule).toBe(APPROVAL_APPROVER_NOT_MEMBERSHIP_TIER); + expect(findings[0].hint).toContain("type: 'position'"); + expect(findings[0].hint).not.toContain('org_membership_level, value'); + }); + it('accepts the position approver type and the other spec types silently', () => { const findings = validateApprovalApprovers(stackWithApprovers([ { type: 'position', value: 'sales_manager' }, diff --git a/packages/lint/src/validate-approval-approvers.ts b/packages/lint/src/validate-approval-approvers.ts index 32a1bc2523..80717dcebe 100644 --- a/packages/lint/src/validate-approval-approvers.ts +++ b/packages/lint/src/validate-approval-approvers.ts @@ -3,23 +3,29 @@ /** * Approval-node approver authoring lint (ADR-0090 D3 fallout). * - * The `role` approver type resolves against better-auth's org-membership tier - * (`sys_member.role`: owner / admin / member) — it is NOT a position. After - * ADR-0090 D3 renamed `sys_role` → `sys_position`, downstream apps that - * authored `{ type: 'role', value: 'sales_manager' }` silently route the - * approval to nobody: the expansion finds no member row, falls back to the - * `role:sales_manager` literal, and the request waits on an approver that can - * never act. This rule moves that failure from a stuck request at runtime to - * a located fix-it at author time. + * `org_membership_level` (and `role`, its deprecated spelling) resolves against + * better-auth's org-membership tier (`sys_member.role`: owner / admin / member) + * — it is NOT a position. After ADR-0090 D3 renamed `sys_role` → `sys_position`, + * downstream apps that authored `{ type: 'role', value: 'sales_manager' }` + * silently route the approval to nobody: the expansion finds no member row, + * falls back to the `role:sales_manager` literal, and the request waits on an + * approver that can never act. These rules move that failure from a stuck + * request at runtime to a located fix-it at author time. * * Rules: * * | Rule | Severity | Origin | * |--------------------------------------------|----------|----------------------------| - * | approval-role-not-membership-tier | warning | ADR-0090 D3 (hotcrm class) | + * | approval-approver-not-membership-tier | warning | ADR-0090 D3 (hotcrm class) | + * | approval-approver-type-deprecated | warning | ADR-0090 D3 (#3133) | * | approval-approver-type-unknown | warning | contract-first (PD #12) | * | approval-escalation-reassign-no-target | warning | silent notify degradation | * + * The first two are mutually exclusive by construction — a bad *value* wins, + * because its fix (`position`) differs from the deprecation's fix + * (`org_membership_level`), and prescribing the latter for a position name + * would be wrong advice. + * * Warnings (not errors): a custom better-auth membership tier is legal, and * the runtime keeps its literal fallback — but both shapes are near-certainly * authoring mistakes, so say it out loud. @@ -27,9 +33,15 @@ * Pure `(stack) => Finding[]`; accepts the NORMALIZED stack input. */ -import { ApproverType, APPROVAL_NODE_TYPE } from '@objectstack/spec/automation'; +import { + ApproverType, + APPROVAL_NODE_TYPE, + DEPRECATED_APPROVER_TYPES, + canonicalApproverType, +} from '@objectstack/spec/automation'; -export const APPROVAL_ROLE_NOT_MEMBERSHIP_TIER = 'approval-role-not-membership-tier'; +export const APPROVAL_APPROVER_NOT_MEMBERSHIP_TIER = 'approval-approver-not-membership-tier'; +export const APPROVAL_APPROVER_TYPE_DEPRECATED = 'approval-approver-type-deprecated'; export const APPROVAL_APPROVER_TYPE_UNKNOWN = 'approval-approver-type-unknown'; export const APPROVAL_ESCALATION_REASSIGN_NO_TARGET = 'approval-escalation-reassign-no-target'; @@ -54,7 +66,8 @@ type AnyRec = Record; /** * The better-auth org-membership tiers `sys_member.role` actually stores * (see `identity/organization.zod.ts` + `mapMembershipRole`). Anything else - * authored as `{ type: 'role' }` is almost certainly a position name. + * authored as `{ type: 'org_membership_level' }` (or its deprecated `role` + * spelling) is almost certainly a position name. */ const MEMBERSHIP_TIERS = new Set(['owner', 'admin', 'member', 'guest']); @@ -121,20 +134,39 @@ export function validateApprovalApprovers(stack: AnyRec): ApprovalApproverFindin continue; } - if (type === 'role' && value && !MEMBERSHIP_TIERS.has(value.toLowerCase())) { + const canonical = canonicalApproverType(type); + + // Exactly one of the two below fires. Order matters: a bad VALUE is + // the more serious (and differently-fixed) defect, so it wins. Telling + // an author to rewrite { type: 'role', value: 'sales_manager' } as + // `org_membership_level` would be actively wrong advice — the fix is + // `position`, and the deprecation is beside the point. + if (canonical === 'org_membership_level' && value && !MEMBERSHIP_TIERS.has(value.toLowerCase())) { findings.push({ severity: 'warning', - rule: APPROVAL_ROLE_NOT_MEMBERSHIP_TIER, + rule: APPROVAL_APPROVER_NOT_MEMBERSHIP_TIER, where, path: `${path}.value`, message: - `approver { type: 'role', value: '${value}' } resolves against the better-auth ` + + `approver { type: '${type}', value: '${value}' } resolves against the better-auth ` + `org-membership tier (sys_member.role: owner/admin/member) — '${value}' is not a ` + `membership tier, so this approver matches nobody and the request stalls.`, hint: `If '${value}' is an org position, author { type: 'position', value: '${value}' } ` + - `(resolved via sys_user_position, ADR-0090 D3). Keep type 'role' only for ` + - `membership tiers (owner/admin/member).`, + `(resolved via sys_user_position, ADR-0090 D3). Keep type 'org_membership_level' ` + + `only for membership tiers (owner/admin/member).`, + }); + } else if (type in DEPRECATED_APPROVER_TYPES) { + const fix = canonicalApproverType(type); + findings.push({ + severity: 'warning', + rule: APPROVAL_APPROVER_TYPE_DEPRECATED, + where, + path: `${path}.type`, + message: + `approver type '${type}' is the deprecated spelling of '${fix}' (ADR-0090 D3) and ` + + `is removed in the next major.`, + hint: `Author { type: '${fix}', value: '${value}' }. It resolves identically today.`, }); } } diff --git a/packages/plugins/plugin-approvals/src/approval-service.test.ts b/packages/plugins/plugin-approvals/src/approval-service.test.ts index 55078feb1a..7e78aa68ab 100644 --- a/packages/plugins/plugin-approvals/src/approval-service.test.ts +++ b/packages/plugins/plugin-approvals/src/approval-service.test.ts @@ -227,6 +227,57 @@ describe('ApprovalService (node era)', () => { expect(req.pending_approvers).toEqual(['position:sales_manager']); }); + // ── approver expansion: org_membership_level + its deprecated `role` alias + // (ADR-0090 D3) ──────────────────────────────────────────────────────── + + // `recordId` is parameterised: the service rejects a second pending request + // on the same record, and the alias test deliberately opens two. + const tierInput = (type: 'org_membership_level' | 'role', recordId = 'opp1') => ({ + ...openInput([]), + recordId, + record: { id: recordId, amount: 100 }, + config: { + approvers: [{ type: type as any, value: 'admin' }], + behavior: 'first_response' as const, + lockRecord: true, + }, + }); + + it('org_membership_level approver: expands the better-auth tier, org-scoped', async () => { + engine._tables['sys_member'] = [ + { id: 'm1', user_id: 'u1', role: 'admin', organization_id: 't1' }, + { id: 'm2', user_id: 'u2', role: 'admin', organization_id: 't1' }, + { id: 'm3', user_id: 'u3', role: 'admin', organization_id: 't2' }, // other tenant + { id: 'm4', user_id: 'u4', role: 'member', organization_id: 't1' }, // other tier + ]; + const req = await svc.openNodeRequest(tierInput('org_membership_level'), CTX); + expect(req.pending_approvers.sort()).toEqual(['u1', 'u2']); + }); + + it('deprecated `role` alias resolves IDENTICALLY to org_membership_level', async () => { + engine._tables['sys_member'] = [ + { id: 'm1', user_id: 'u1', role: 'admin', organization_id: 't1' }, + { id: 'm2', user_id: 'u4', role: 'member', organization_id: 't1' }, + ]; + const canonical = await svc.openNodeRequest(tierInput('org_membership_level', 'opp_canon'), CTX); + const deprecated = await svc.openNodeRequest(tierInput('role', 'opp_depr'), CTX); + expect(deprecated.pending_approvers).toEqual(canonical.pending_approvers); + expect(deprecated.pending_approvers).toEqual(['u1']); + }); + + // The fallback literal keeps the AUTHORED spelling: `sys_approval_approver` + // rows and `pending_approvers` slots written by 15.x carry `role:`, and + // canonicalising the literal here would orphan every one of them. + it('deprecated `role` alias keeps its legacy literal on fallback (no orphaned slots)', async () => { + const req = await svc.openNodeRequest(tierInput('role'), CTX); + expect(req.pending_approvers).toEqual(['role:admin']); + }); + + it('org_membership_level falls back to its own canonical literal', async () => { + const req = await svc.openNodeRequest(tierInput('org_membership_level'), CTX); + expect(req.pending_approvers).toEqual(['org_membership_level:admin']); + }); + it("department approver: honors the spec enum value 'department' (not just the business_unit dialect)", async () => { engine._tables['sys_business_unit'] = [ { id: 'bu1', organization_id: 't1', active: true }, diff --git a/packages/plugins/plugin-approvals/src/approval-service.ts b/packages/plugins/plugin-approvals/src/approval-service.ts index 5a389368de..dcfce07cae 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, + canonicalApproverType, type ApprovalNodeConfig, } from '@objectstack/spec/automation'; import type { @@ -274,10 +275,10 @@ export class ApprovalService implements IApprovalService { /** * Expand the approvers on an Approval node into user IDs by querying the - * graph tables for `team:` / `department:` / `position:` / `role:` / - * `manager:` approver types. Falls back to a prefixed literal - * (`type:value`) when graph lookups produce nothing — so existing fixtures - * and flows that rely on substring matching keep working. + * graph tables for `team:` / `department:` / `position:` / + * `org_membership_level:` / `manager:` approver types. Falls back to a + * prefixed literal (`type:value`) when graph lookups produce nothing — so + * existing fixtures and flows that rely on substring matching keep working. * * **Graph semantics:** * - `team` → flat members of `sys_team` (better-auth; no BFS) @@ -285,34 +286,50 @@ export class ApprovalService implements IApprovalService { * → members of every descendant via `sys_business_unit_member` * - `position` → holders via `sys_user_position` ∪ `sys_member.role` * transition source (ADR-0090 D3 / ADR-0057 D4) - * - `role` → users with `sys_member.role = value` in tenant — the + * - `org_membership_level` + * → users with `sys_member.role = value` in tenant — the * better-auth MEMBERSHIP TIER (owner/admin/member), not a * position; author `position` for org positions * - `manager` → `sys_user.manager_id` of `record[value] ?? record.owner_id` * - `field` → literal user id stored in `record[value]` * - `user` → literal value + * + * `role` is accepted as the deprecated spelling of `org_membership_level` + * (ADR-0090 D3) for one window: it resolves identically and logs a warning. */ private async expandApprovers(step: any, record?: any, organizationId?: string | null): Promise { if (!step || !Array.isArray(step.approvers)) return []; const out: string[] = []; for (const a of step.approvers) { if (!a) continue; - if (a.type === 'user') { out.push(String(a.value)); continue; } - if (a.type === 'field' && record) { out.push(String((record as any)[a.value] ?? '')); continue; } + // ADR-0090 D3: `role` is the deprecated spelling of + // `org_membership_level`. Resolve on the canonical type, but keep the + // AUTHORED spelling in the `type:value` fallback below — stored + // `sys_approval_approver` rows and `pending_approvers` slots from 15.x + // carry the old literal, and rewriting it here would orphan them. + const type = canonicalApproverType(String(a.type)); + if (type !== a.type) { + this.logger?.warn?.( + `[approvals] approver type '${a.type}' is deprecated (ADR-0090 D3) — author '${type}' instead`, + { deprecated: a.type, canonical: type }, + ); + } + if (type === 'user') { out.push(String(a.value)); continue; } + if (type === 'field' && record) { out.push(String((record as any)[a.value] ?? '')); continue; } try { - if (a.type === 'team') { + if (type === 'team') { const users = await this.expandTeamUsers(String(a.value)); if (users.length) { for (const u of users) out.push(u); continue; } - } else if (a.type === 'department' || a.type === 'business_unit' || a.type === 'bu') { + } else if (type === 'department' || type === 'business_unit' || type === 'bu') { const users = await this.expandBusinessUnitUsers(String(a.value), organizationId); if (users.length) { for (const u of users) out.push(u); continue; } - } else if (a.type === 'position') { + } else if (type === 'position') { const users = await this.expandPositionUsers(String(a.value), organizationId); if (users.length) { for (const u of users) out.push(u); continue; } - } else if (a.type === 'role') { - const users = await this.expandRoleUsers(String(a.value), organizationId); + } else if (type === 'org_membership_level') { + const users = await this.expandMembershipTierUsers(String(a.value), organizationId); if (users.length) { for (const u of users) out.push(u); continue; } - } else if (a.type === 'manager' && record) { + } else if (type === 'manager' && record) { const subject = (record as any)[a.value] ?? (record as any).owner_id; if (subject) { const mgr = await this.lookupManager(String(subject)); @@ -406,14 +423,23 @@ export class ApprovalService implements IApprovalService { if (uid) users.add(uid); } } catch { /* table may not exist on minimal stacks — union source below still applies */ } - for (const uid of await this.expandRoleUsers(positionName, organizationId)) users.add(uid); + // ADR-0057 D4 transition source: pre-migration stacks still carry the + // position name in better-auth's `sys_member.role` column, so the same + // lookup serves a position name here and a membership tier for + // `org_membership_level` — the column is one, the two concepts are not. + for (const uid of await this.expandMembershipTierUsers(positionName, organizationId)) users.add(uid); return Array.from(users); } - /** better-auth org-membership tier (`sys_member.role`) — NOT positions. */ - private async expandRoleUsers(roleName: string, organizationId?: string | null): Promise { - if (!roleName) return []; - const filter: any = { role: roleName }; + /** + * better-auth org-membership tier (`sys_member.role`: owner/admin/member) — + * NOT positions. Named for the projection (`org_membership_level`, ADR-0057 + * D7 / ADR-0090 D3), not for better-auth's column: the column name is theirs + * and stays, the platform-facing word does not. + */ + private async expandMembershipTierUsers(tier: string, organizationId?: string | null): Promise { + if (!tier) return []; + const filter: any = { role: tier }; if (organizationId) filter.organization_id = organizationId; let rows: any[] = []; try { diff --git a/packages/spec/api-surface.json b/packages/spec/api-surface.json index 3cd9a70a34..dd8f38aa9d 100644 --- a/packages/spec/api-surface.json +++ b/packages/spec/api-surface.json @@ -1997,6 +1997,7 @@ "ConnectorSchema (const)", "ConnectorTrigger (type)", "ConnectorTriggerSchema (const)", + "DEPRECATED_APPROVER_TYPES (const)", "DataDestinationConfig (type)", "DataDestinationConfigSchema (const)", "DataSourceConfig (type)", @@ -2116,6 +2117,7 @@ "WebhookSchema (const)", "WebhookTriggerType (const)", "analyzeRegion (function)", + "canonicalApproverType (function)", "defineActionDescriptor (function)", "defineFlow (function)", "defineWebhook (function)", diff --git a/packages/spec/src/automation/approval.test.ts b/packages/spec/src/automation/approval.test.ts index f3733c02c8..84e9927e14 100644 --- a/packages/spec/src/automation/approval.test.ts +++ b/packages/spec/src/automation/approval.test.ts @@ -1,17 +1,20 @@ import { describe, it, expect } from 'vitest'; import { ApproverType, + DEPRECATED_APPROVER_TYPES, + canonicalApproverType, APPROVAL_NODE_TYPE, ApprovalDecision, APPROVAL_BRANCH_LABELS, ApprovalNodeApproverSchema, ApprovalEscalationSchema, ApprovalNodeConfigSchema, + getApprovalNodeConfigJsonSchema, } from './approval.zod'; describe('ApproverType', () => { it('should accept all valid approver types', () => { - ['user', 'role', 'position', 'team', 'department', 'manager', 'field', 'queue'].forEach(t => { + ['user', 'org_membership_level', 'position', 'team', 'department', 'manager', 'field', 'queue'].forEach(t => { expect(() => ApproverType.parse(t)).not.toThrow(); }); }); @@ -19,6 +22,39 @@ describe('ApproverType', () => { it('should reject invalid approver type', () => { expect(() => ApproverType.parse('group')).toThrow(); }); + + // ADR-0090 D3: `role` is the pre-relabel spelling of `org_membership_level`. + // It stays parseable for one deprecation window so a stored 15.x flow keeps + // loading; the runtime warns and `os lint` prescribes the rewrite. + it('still accepts the deprecated `role` spelling during its window', () => { + expect(() => ApproverType.parse('role')).not.toThrow(); + }); + + it('canonicalises the deprecated spelling, passes others through', () => { + expect(DEPRECATED_APPROVER_TYPES.role).toBe('org_membership_level'); + expect(canonicalApproverType('role')).toBe('org_membership_level'); + expect(canonicalApproverType('position')).toBe('position'); + expect(canonicalApproverType('user')).toBe('user'); + // Every canonical target must itself be a member of the enum, or the + // rewrite the lint prescribes would not parse. + for (const target of Object.values(DEPRECATED_APPROVER_TYPES)) { + expect(() => ApproverType.parse(target)).not.toThrow(); + } + }); + + // Cross-repo contract: the published node configSchema must carry + // `xEnumDeprecated` on the approver type, or the Studio designer (objectui) + // derives its dropdown straight from `enum` and keeps offering `role` — the + // exact trap ADR-0090 D3 retires. Renderers read this to omit deprecated + // members from pickers while still rendering a stored value. + it('publishes xEnumDeprecated on the approver type so pickers can drop `role`', () => { + const schema = getApprovalNodeConfigJsonSchema() as any; + const typeNode = schema?.properties?.approvers?.items?.properties?.type; + expect(typeNode?.enum).toContain('role'); // still parses (back-compat) + expect(typeNode?.enum).toContain('org_membership_level'); + expect(typeNode?.xEnumDeprecated).toEqual(Object.keys(DEPRECATED_APPROVER_TYPES)); + expect(typeNode?.xEnumDeprecated).toContain('role'); + }); }); describe('Approval node constants (ADR-0019)', () => { diff --git a/packages/spec/src/automation/approval.zod.ts b/packages/spec/src/automation/approval.zod.ts index 22835049c2..d9169b9fdb 100644 --- a/packages/spec/src/automation/approval.zod.ts +++ b/packages/spec/src/automation/approval.zod.ts @@ -8,9 +8,18 @@ import { lazySchema } from '../shared/lazy-schema'; */ export const ApproverType = z.enum([ 'user', // Specific user(s) - // `role` is the better-auth ORG-MEMBERSHIP TIER (sys_member.role: owner / - // admin / member) — NOT an org position. Post ADR-0090 D3 a value like - // 'sales_manager' here silently matches nobody; author `position` instead. + // 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 + // 'organization membership', never 'role'"). NOT an org position: a value + // like 'sales_manager' matches nobody — author `position` for those. + 'org_membership_level', + // @deprecated ADR-0090 D3 — the pre-relabel spelling of + // `org_membership_level`. D3 makes "role" reserved-forbidden on platform + // 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. 'role', 'position', // Holders of a position (sys_user_position, ADR-0090 D3) 'team', // Members of a flat collaboration team (sys_team) @@ -20,6 +29,20 @@ export const ApproverType = z.enum([ 'queue' // Data ownership queue ]); +/** + * Deprecated approver-type spellings → their canonical replacement + * (ADR-0090 D3). The runtime and `os lint` both read this map, so a future + * removal is a one-line edit here plus the enum entry. + */ +export const DEPRECATED_APPROVER_TYPES = { + role: 'org_membership_level', +} as const satisfies Record>; + +/** Resolve a possibly-deprecated approver type to its canonical spelling. */ +export function canonicalApproverType(type: string): string { + return (DEPRECATED_APPROVER_TYPES as Record)[type] ?? type; +} + // ========================================================================== // Approval as a Flow Node (ADR-0019, canonical) // ========================================================================== @@ -76,21 +99,32 @@ export const APPROVAL_BRANCH_LABELS = { /** A single approver assignment on an Approval node. */ export const ApprovalNodeApproverSchema = lazySchema(() => z.object({ - type: ApproverType, + // `xEnumDeprecated` lists enum members that still PARSE but must not be + // offered for new authoring. Without it the Studio designer derives its + // approver-type dropdown straight from this enum and keeps offering `role` + // — the exact trap ADR-0090 D3 is retiring — one click away from `position`. + // Renderers omit these from pickers while still rendering a stored value. + type: ApproverType.meta({ + xEnumDeprecated: Object.keys(DEPRECATED_APPROVER_TYPES), + }), /** * The approver reference, interpreted per `type`: a user id (`user`), a - * membership tier — owner/admin/member (`role`), a position machine name - * (`position`), team/department id (`team`/`department`), field name - * holding a user id (`field`), or queue id (`queue`). Omitted for `manager` - * (resolved from the submitter's `manager_id`). + * membership tier — owner/admin/member (`org_membership_level`), a position + * machine name (`position`), team/department id (`team`/`department`), field + * name holding a user id (`field`), or queue id (`queue`). Omitted for + * `manager` (resolved from the submitter's `manager_id`). */ // `xRef` marks this string as a *polymorphic* typed reference (ADR-0018 // §configSchema): the concrete picker follows the sibling `type` column, so - // the Studio designer shows a user/role/position/team/department/queue - // picker — or an object-field picker (resolved from the flow's `$trigger` - // object) when `type` is `field`. `manager` and any unmapped value carry no - // `value` and stay free text. A single `.meta()` carries both description - // and annotation. + // the Studio designer shows a user/membership-tier/position/team/department/ + // queue picker — or an object-field picker (resolved from the flow's + // `$trigger` object) when `type` is `field`. `manager` and any unmapped + // value carry no `value` and stay free text. A single `.meta()` carries both + // description and annotation. + // + // The `role` → `org-membership-level` picker kind is the deprecated alias's + // entry: it maps to the SAME picker as the canonical spelling, so a stored + // legacy node still renders correctly for its deprecation window. value: z.string().optional().meta({ description: 'User id / membership tier / position / team / department / field / queue — per `type`', xRef: { @@ -98,7 +132,8 @@ export const ApprovalNodeApproverSchema = lazySchema(() => z.object({ objectSource: '$trigger', map: { user: 'user', - role: 'role', + org_membership_level: 'org-membership-level', + role: 'org-membership-level', position: 'position', team: 'team', department: 'department', diff --git a/scripts/role-word-baseline.json b/scripts/role-word-baseline.json index ebcfd33018..5d11bb70ea 100644 --- a/scripts/role-word-baseline.json +++ b/scripts/role-word-baseline.json @@ -3,7 +3,7 @@ "content/docs/ai/index.mdx": 2, "content/docs/api/error-catalog.mdx": 1, "content/docs/api/wire-format.mdx": 1, - "content/docs/automation/approvals.mdx": 5, + "content/docs/automation/approvals.mdx": 1, "content/docs/automation/hooks.mdx": 1, "content/docs/concepts/architecture.mdx": 4, "content/docs/concepts/metadata-lifecycle.mdx": 1, @@ -42,7 +42,7 @@ "content/docs/ui/forms.mdx": 3, "skills/objectstack-ai/SKILL.md": 8, "skills/objectstack-api/SKILL.md": 1, - "skills/objectstack-automation/SKILL.md": 3, + "skills/objectstack-automation/SKILL.md": 1, "skills/objectstack-data/SKILL.md": 8, "skills/objectstack-data/rules/relationships.md": 1, "skills/objectstack-platform/SKILL.md": 3, diff --git a/skills/objectstack-automation/SKILL.md b/skills/objectstack-automation/SKILL.md index dc5ad4eff5..bcac753055 100644 --- a/skills/objectstack-automation/SKILL.md +++ b/skills/objectstack-automation/SKILL.md @@ -377,7 +377,7 @@ branch — you never resume the flow by hand. |:-------|:------------| | `user` | A specific user id (`value` = user id) | | `position` | Holders of a position — `value` = the position machine name, resolved via `sys_user_position` (ADR-0090 D3) | -| `role` | The better-auth **org-membership tier** (`sys_member.role`: `owner`/`admin`/`member`) — **NOT** a position. `{ type: 'role', value: 'sales_manager' }` matches nobody; use `position` | +| `org_membership_level` | The better-auth **org-membership tier** — `value` is one of `owner`/`admin`/`member`, and nothing else. **NOT** a position: `{ type: 'org_membership_level', value: 'sales_manager' }` matches nobody; use `position`. Spelled `role` before ADR-0090 D3 — that spelling is deprecated, still resolves, and is removed in the next major | | `team` | Members of a flat `sys_team` | | `department` | A department + all descendant departments | | `manager` | The submitter's manager (`sys_user.manager_id`) |