feat(approvals)!: rename role approver type to org_membership_level — and unbreak main

.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:<v>`, 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`.

content/docs/automation/approvals.mdx

@@ -53,11 +53,14 @@ defineFlow({
 ```
 
 <Callout type="warn">
-**`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.
 </Callout>
 
 Approving is itself a gated action — model "may approve" as a capability (`approve_invoice`) the approver's permission set grants, and gate the approve action's `requiredPermissions` on it so the gate is enforced on **both** the UI and the server (ADR-0066 D4).
@@ -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 `<type>:<value>` 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`.
 

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`

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';

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' },

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

@@ -3,33 +3,45 @@
 /**
  * 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.
  *
  * 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<string, unknown>;
 /**
  * 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.`,
           });
         }
       }

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:<v>`, 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 },