diff --git a/.changeset/21387-retire-role-arm.md b/.changeset/21387-retire-role-arm.md new file mode 100644 index 00000000000..7e3e89263b4 --- /dev/null +++ b/.changeset/21387-retire-role-arm.md @@ -0,0 +1,25 @@ +--- +"@objectstack/plugin-approvals": minor +"@objectstack/spec": patch +--- + +fix(plugin-approvals)!: `role:` is no longer a position address, and the deprecated `role` approver type stops writing `role:` slots (ADR-0090 D3) + +Clause-②: no (narrowing) + +`position:` is now the one spelling of a position address. ADR-0090 D3 retired the word `role` with no alias window; the approvals service still read `role:` as a second spelling of the same position everywhere it compares a slot with the caller ("My Pending", the participant gate, `viewer.can_act`, and the slot test of every decision). The stock console now sends `position:`, so that arm is gone. + +**FROM → TO.** FROM `role:` → TO `position:`, wherever a caller names a position: the `approverId` filter of `GET /api/v1/approvals/requests`, and the `actorId` of approve, reject, send back, reassign, request info and comment. A `role:` ask now matches only a slot stored under that exact spelling, and a `role:` actor is refused with 403 `FORBIDDEN` ("cannot act as …"). + +**The writer.** An approver authored with the deprecated type `{ type: 'role', value: … }` already resolved as `org_membership_level` (the org-membership tier: owner, admin, member). When that lookup found no one, the request's fallback slot kept the authored spelling, `role:`, and a holder of a position with the same name decided it through the `role:` arm. That fallback now writes the canonical `org_membership_level:`, so no path writes a `role:` slot. A stored slot is never rewritten. + +Two classes of pending request are now decided only by an admin override: + +- a request a 15.x-era release opened, whose slot is stored as `role:`; +- a new request opened from a flow that still authors `{ type: 'role', value: '' }` and whose membership-tier lookup finds no one (its slot is `org_membership_level:`). + +**Author's one-line fix:** write `{ type: 'position', value: '' }`. `os lint` already reports the old form as `approval-approver-not-membership-tier` or `approval-approver-type-deprecated`. + +**Admin's one-line handling, both classes:** a platform admin (`admin_full_access`) or a tenant admin of the request's organization approves or rejects it (`POST /api/v1/approvals/requests/:id/approve` or `/reject`; recorded with `via_override: true`, and the flow run resumes), or reassigns it to the position's holder (`POST /api/v1/approvals/requests/:id/reassign` with `{ "to": "" }`), who then decides it normally. + + diff --git a/content/docs/automation/approvals.mdx b/content/docs/automation/approvals.mdx index 50444d4ccf4..0925c9662ca 100644 --- a/content/docs/automation/approvals.mdx +++ b/content/docs/automation/approvals.mdx @@ -61,7 +61,10 @@ different thing — the better-auth **org-membership tier**, whose only values a `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. +(ADR-0090 D3): it still resolves, warns at runtime, and is removed in the next major. It resolves +exactly as `org_membership_level` does, so a position name authored there finds no member and +leaves the `org_membership_level:` slot, which only an admin override can decide. The fix +is one line: `{ type: 'position', value: '' }`. @@ -417,10 +420,11 @@ curl -b cookies.txt \ `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. A position literal matches under both -spellings the service admits a holder of that position under as their own -identity: `position:`, and the deprecated pre-rename prefix the stock -console still sends — so either one finds the same requests. +a person's identities in one call. A position literal matches under +`position:`, the one spelling the service admits a holder of that +position under as their own identity. The deprecated pre-rename prefix is no +alias of it (ADR-0090 D3): it matches only a slot stored under that exact +spelling, and no holder acts under it. Other filters: `status`, `object`, `recordId`, `submitterId`, `q`, `limit`, `offset`. @@ -429,7 +433,7 @@ Other filters: `status`, `object`, `recordId`, `submitterId`, `q`, `limit`, separately: a request is visible to its **participants** — the submitter, a current approver (counted by every identity the decision routes let the caller take a slot under: their user id, the email their own account carries, and -either spelling of a position they hold), and anyone who has already acted on +the `position:` address of a position they hold), and anyone who has already acted on it (a past approver whose slot has moved on, a commenter). An action is recorded under the slot it took, so "already acted" is counted by those same identities: a decision taken on a `position:` slot keeps the request visible to @@ -503,11 +507,13 @@ curl -b cookies.txt -X POST \ `actorId` defaults to the caller, who takes the first slot in `pending_approvers` keyed by one of their identities — their user id, the email -their own account carries, or either spelling of a position they hold (a -`position:` literal left by a position nobody held when the request -opened, once someone is staffed into it). A named `actorId` must be one of those -identities, and a position named under either spelling takes that position's -slot. No such slot and no admin override returns 403 (`FORBIDDEN: actor '…' is +their own account carries, or the `position:` address of a position they +hold (the literal left by a position nobody held when the request opened, once +someone is staffed into it). A named `actorId` must be one of those identities, +and a named `position:` takes that position's slot; the deprecated +pre-rename prefix is not one of them (ADR-0090 D3) and returns 403 +(`FORBIDDEN: cannot act as '…'`). No +such slot and no admin override returns 403 (`FORBIDDEN: actor '…' is not a pending approver`); a request that isn't pending returns 409 (`INVALID_STATE`). The decision records two facts on its `sys_approval_action` row: `actor_id` is the user who decided, and `acted_as` is the slot the decision @@ -624,8 +630,8 @@ service attaches to every request it returns: - `can_act` — the caller is a **current pending approver**: while the request is `pending`, the caller with no `actorId` named would take one of its slots — - under their user id, the email their own account carries, or either spelling - of a position they hold. It is computed by the same function the decision + under their user id, the email their own account carries, or the + `position:` address of a position they hold. It is computed by the same function the decision routes authorize with, so it reflects position/team/manager resolution and a `position:` slot whose position was staffed after the request opened. - `is_submitter` — the caller submitted the request. @@ -655,6 +661,14 @@ authoritative: it finalizes the node even under `unanimous`/`quorum`/`per_group` and is audited under the admin's own id. Prefer a guaranteed-staffed fallback approver so the set is never empty in the first place. +The same override is the only way to decide a slot no position holder addresses +since ADR-0090 D3 retired the pre-rename word: a request a 15.x-era release stored +under the pre-rename prefix, and a request opened from a flow that still authors +the deprecated approver type (above) with a position name as its value (its slot +is `org_membership_level:`). Approve or reject it as an admin, or reassign it +to the position's holder, who then decides it normally; then change the flow to +`{ type: 'position', value: '' }`. + Note the rule is **"the actor is an admin"**, not "the slate is unstaffed" — so an admin can also act on a request whose slate *is* properly staffed, bypassing the people on it. That is why the decision records **which door it came diff --git a/packages/plugins/plugin-approvals/src/approval-service.test.ts b/packages/plugins/plugin-approvals/src/approval-service.test.ts index 4f555706496..56e5c6f8f9e 100644 --- a/packages/plugins/plugin-approvals/src/approval-service.test.ts +++ b/packages/plugins/plugin-approvals/src/approval-service.test.ts @@ -129,6 +129,26 @@ function makeFakeEngine() { const CTX = { userId: 'u1', tenantId: 't1', positions: [], permissions: [] } as any; const SYS = { isSystem: true, positions: [], permissions: [] } as any; +/** + * Store an open request's slot the way a 15.x-era request stored it: + * `role:

`, the spelling ADR-0090 D3 retired, in the CSV source of truth — + * then let the plugin's own boot backfill (`rebuildApproverIndex`) bring the + * index in line, as it does on an upgraded deployment. No current writer + * produces the spelling, so a pin on a stored slot has to plant it. + */ +async function storeLegacyRoleSlot( + engine: ReturnType, + svc: ApprovalService, + requestId: string, + slot = 'role:sales_manager', +): Promise { + await engine.update('sys_approval_request', { id: requestId, pending_approvers: slot }); + await svc.rebuildApproverIndex(); + const indexed = (engine._tables['sys_approval_approver'] ?? []).filter(r => r.request_id === requestId); + // The plant landed — a pin on a slot that is not there would pass vacuously. + expect(indexed.map(r => r.approver)).toEqual([slot]); +} + /** * Every session shape that could plausibly be read as "this caller is an admin" * — pinned as NOT exempt by the guards in `lifecycle-hooks.ts` (#4839). @@ -896,12 +916,15 @@ describe('ApprovalService (node era)', () => { 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 () => { + // The fallback literal is written in the CANONICAL type (ADR-0090 D3): no + // path writes a `role:` slot, a spelling no reader addresses. A slot a + // 15.x-era request stored as `role:` stays as stored — nothing rewrites + // it — and is decided by the privileged override (pinned in the + // `role:` retirement block below). + it('deprecated `role` alias falls back to the canonical literal, in the slate and in the index', async () => { const req = await svc.openNodeRequest(tierInput('role'), CTX); - expect(req.pending_approvers).toEqual(['role:admin']); + expect(req.pending_approvers).toEqual(['org_membership_level:admin']); + expect((engine._tables['sys_approval_approver'] ?? []).map(r => r.approver)).toEqual(['org_membership_level:admin']); }); it('org_membership_level falls back to its own canonical literal', async () => { @@ -3602,12 +3625,13 @@ describe('ApprovalService — participant visibility (#3590)', () => { // ── "My Pending" reads the acting path's position addresses (#21350) ─── // // A request routed to a position nobody held when it opened keeps the literal -// `position:

` slot (a 15.x-era one reads `role:

`), and the console asks -// "My Pending" under `role:

`. `resolveActor` has always admitted a holder of -// `p` under either spelling, but the list filter matched the caller's -// `approverId` literally and the participant gate keyed on the user id alone — -// so the request was missing from the inbox of the very user who could decide -// it. All three now read ONE equivalence (`approver-address.ts`). +// `position:

` slot. The list filter used to match the caller's `approverId` +// literally and the participant gate keyed on the user id alone — so the +// request was missing from the inbox of the very user who could decide it. All +// three now read ONE equivalence (`approver-address.ts`), whose one position +// spelling is `position:

`: ADR-0090 D3 retired `role:

`, so a `role:` ask +// finds nothing, and a slot a 15.x-era request stored under it is decided by +// the privileged override (the retirement block below). describe('ApprovalService — "My Pending" position addresses (#21350)', () => { const svcFor = (engine: any) => { let n = 0; @@ -3636,33 +3660,42 @@ describe('ApprovalService — "My Pending" position addresses (#21350)', () => { return out; }; - it('a holder of the position finds the request under EITHER spelling — list, count and the request itself', async () => { + it('a holder of the position finds the request under `position:` — list, count and the request itself — and no longer under `role:`', async () => { const engine = makeFakeEngine(); const svc = svcFor(engine); const req = await open(svc, routedTo('position')); // submitter u1 expect(req.pending_approvers).toEqual(['position:sales_manager']); - for (const spelling of SPELLINGS) { - // The console's own identity list: user id, then the position address. - const filter = { status: 'pending' as const, approverId: ['u_reviewer', spelling] }; - const listed = await svc.listRequests(filter, REVIEWER); - expect(listed.map(r => r.id), `listed under '${spelling}'`).toEqual([req.id]); - expect(await svc.countRequests(filter, REVIEWER), `counted under '${spelling}'`).toBe(1); - } + // The console's own identity list: user id, then the position address. + const filter = { status: 'pending' as const, approverId: ['u_reviewer', 'position:sales_manager'] }; + expect((await svc.listRequests(filter, REVIEWER)).map(r => r.id)).toEqual([req.id]); + expect(await svc.countRequests(filter, REVIEWER)).toBe(1); expect(await svc.getRequest(req.id, REVIEWER)).not.toBeNull(); + + // ADR-0090 D3: the retired spelling is no ask for the same slot. + const retired = { status: 'pending' as const, approverId: ['u_reviewer', 'role:sales_manager'] }; + expect(await svc.listRequests(retired, REVIEWER), 'listed under role:').toEqual([]); + expect(await svc.countRequests(retired, REVIEWER), 'counted under role:').toBe(0); }); - it('a 15.x-era `role:` slot is found under the `position:` spelling too', async () => { + it('a stored 15.x-era `role:` slot is no position address: not found under `position:`, hidden from the holder; the deprecated type writes no such slot any more', async () => { const engine = makeFakeEngine(); const svc = svcFor(engine); - // `role` is the deprecated approver type; its literal keeps the authored spelling. - const req = await open(svc, routedTo('role')); - expect(req.pending_approvers).toEqual(['role:sales_manager']); + const req = await open(svc, routedTo('position')); + await storeLegacyRoleSlot(engine, svc, req.id); + // SYS sees every row: the slot is there, under its own spelling only. + expect((await svc.listRequests({ approverId: 'role:sales_manager' }, SYS)).map(r => r.id)).toEqual([req.id]); + expect(await svc.listRequests({ approverId: 'position:sales_manager' }, SYS)).toEqual([]); for (const spelling of SPELLINGS) { const listed = await svc.listRequests({ status: 'pending', approverId: ['u_reviewer', spelling] }, REVIEWER); - expect(listed.map(r => r.id), `listed under '${spelling}'`).toEqual([req.id]); + expect(listed, `listed under '${spelling}'`).toEqual([]); } + expect(await svc.getRequest(req.id, REVIEWER)).toBeNull(); + + // The deprecated approver TYPE now falls back to the canonical literal. + const fresh = await open(svc, { ...routedTo('role'), recordId: 'opp2', runId: 'run_2', record: { id: 'opp2', amount: 100 } }); + expect(fresh.pending_approvers).toEqual(['org_membership_level:sales_manager']); }); it('negative control: a user who does not hold the position sees nothing, under either spelling', async () => { @@ -3685,8 +3718,8 @@ describe('ApprovalService — "My Pending" position addresses (#21350)', () => { const req = await open(svc, routedTo('position')); // SYS sees every row, so a miss here is the FILTER's verdict alone. - expect((await svc.listRequests({ approverId: 'role:sales_manager' }, SYS)).map(r => r.id)).toEqual([req.id]); - for (const address of ['team:sales_manager', 'org_membership_level:sales_manager', 'sales_manager']) { + expect((await svc.listRequests({ approverId: 'position:sales_manager' }, SYS)).map(r => r.id)).toEqual([req.id]); + for (const address of ['role:sales_manager', 'team:sales_manager', 'org_membership_level:sales_manager', 'sales_manager']) { expect(await svc.listRequests({ approverId: address }, SYS), `'${address}' must not fold`).toEqual([]); } }); @@ -3705,14 +3738,16 @@ describe('ApprovalService — "My Pending" position addresses (#21350)', () => { expect(out.request.status).toBe('approved'); }); - // `resolveActor` must admit EXACTLY what it admitted before the equivalence - // moved into `approver-address.ts`. The oracle is the pre-extraction - // predicate, verbatim; the matrix crosses both spellings with near misses and - // with position names that contain a prefix themselves. - it('resolveActor admits exactly the identities the pre-extraction predicate admitted', async () => { + // `resolveActor` must admit EXACTLY what the pre-extraction predicate + // admitted, minus its retired `role:` arm (ADR-0090 D3). The oracle is that + // predicate with the arm removed; the matrix crosses both spellings with near + // misses and with position names that contain a prefix themselves, so every + // `role:` row is a refusal and every `position:` row of a held position an + // admission. + it('resolveActor admits exactly `position:

` for a held position — the pre-extraction predicate without its role: arm', async () => { const svc = svcFor(makeFakeEngine()); const preExtraction = (named: string, positions: unknown[]) => - positions.some((position) => named === `position:${position}` || named === `role:${position}`); + positions.some((position) => named === `position:${position}`); const NAMED = [ 'position:cfo', 'role:cfo', 'team:cfo', 'org_membership_level:cfo', 'positions:cfo', 'Position:cfo', 'cfo', 'position:', 'role:', 'position:role:cfo', 'role:position:cfo', 'position:cfo ', 'u_other', @@ -3730,8 +3765,9 @@ describe('ApprovalService — "My Pending" position addresses (#21350)', () => { if (admitted) admittedCount++; } } - // Both arms of the matrix are exercised — not a sweep of refusals only. - expect(admittedCount).toBeGreaterThan(5); + // Both arms of the matrix are exercised — not a sweep of refusals only: + // position:cfo twice, position:role:cfo, position: and position:7. + expect(admittedCount).toBe(5); }); }); @@ -3743,8 +3779,8 @@ describe('ApprovalService — "My Pending" position addresses (#21350)', () => { // slot test, the already-acted probe and — for a `user` approver authored as // an email — the participant gate's email half. A holder of a position whose // slot reads `position:

` therefore saw the request with `can_act: false`, -// was refused with the default actor and with the console's `role:

`, -// could decide it only by naming `position:

`, and lost sight of it after. +// was refused with the default actor, could decide it only by naming +// `position:

`, and lost sight of it after. // One pin per reader below; `approver-address-readers.test.ts` enumerates // them and fails on a slot-against-caller comparison written anywhere else. describe('ApprovalService — every slot reader takes the acting addresses (#21379)', () => { @@ -3821,10 +3857,10 @@ describe('ApprovalService — every slot reader takes the acting addresses (#213 } }); - it('decision slot test: the default actor and BOTH spellings take the position slot; the decision records the person in actor_id and the slot\'s stored spelling in acted_as', async () => { + it('decision slot test: the default actor and `position:` take the position slot, `role:` does not; the decision records the person in actor_id and the slot\'s stored spelling in acted_as', async () => { const engine = makeFakeEngine(); const svc = svcFor(engine); - for (const actorId of [undefined, 'role:sales_manager', 'position:sales_manager']) { + for (const actorId of [undefined, 'position:sales_manager']) { const req = await open(svc, toPosition); const out = await svc.decideNode(req.id, { decision: 'approve', actorId } as any, HOLDER); expect(out.finalized, `actor ${actorId ?? '(default)'}`).toBe(true); @@ -3836,12 +3872,20 @@ describe('ApprovalService — every slot reader takes the acting addresses (#213 `recorded for actor ${actorId ?? '(default)'}`, ).toEqual(['u_holder', SLOT, false]); } - // A 15.x-era slot keeps its own spelling, under the default actor too. - const legacy = await open(svc, [{ type: 'role', value: 'sales_manager' }]); - expect(legacy.pending_approvers).toEqual(['role:sales_manager']); - await svc.decideNode(legacy.id, { decision: 'reject' } as any, HOLDER); - const [rejected] = actionsOf(engine, legacy.id, 'reject'); - expect([rejected.actor_id, rejected.acted_as]).toEqual(['u_holder', 'role:sales_manager']); + // ADR-0090 D3: the holder naming the retired spelling is refused — it is + // no identity the server can prove — and nothing is recorded. + const named = await open(svc, toPosition); + await expect(svc.decideNode(named.id, { decision: 'approve', actorId: 'role:sales_manager' }, HOLDER)) + .rejects.toThrow(/^FORBIDDEN: cannot act as 'role:sales_manager'/); + expect(actionsOf(engine, named.id, 'approve')).toEqual([]); + + // A stored 15.x-era `role:` slot is no position address: its holder takes + // nothing there, under the default actor either. + const legacy = await open(svc, toPosition); + await storeLegacyRoleSlot(engine, svc, legacy.id); + await expect(svc.decideNode(legacy.id, { decision: 'reject' } as any, HOLDER)) + .rejects.toThrow("FORBIDDEN: actor 'u_holder' is not a pending approver"); + expect(actionsOf(engine, legacy.id, 'reject')).toEqual([]); // This widens nobody: holding A position is not holding THIS one. const req = await open(svc, toPosition); @@ -3954,6 +3998,162 @@ describe('ApprovalService — every slot reader takes the acting addresses (#213 }); }); +// ── The `role:` arm retired (ADR-0090 D3) ─────────────────────────────── +// +// `position:

` is the ONE spelling of a position address: `role:

` is no +// alias of it, and the deprecated `role` approver TYPE writes its canonical +// `org_membership_level:` literal when its membership-tier lookup finds +// nobody, so no path writes a `role:` slot. Two classes of request are left +// to the privileged override (or a reassign to a real approver), and the +// ruling accepted both: a slot a 15.x-era request stored as `role:

`, and a +// new request from a flow that still authors `{ type: 'role', value: }`. These pin the holder's half — the position slot, listed, +// `can_act` and decided with the default actor; `role:

` no address of it — +// and the rescue, end to end through `decide()`, for both classes. +describe('ApprovalService — the `role:` arm retired; the override decides what it leaves (ADR-0090 D3)', () => { + const svcFor = (engine: any) => { + let n = 0; + return new ApprovalService({ engine, clock: { now: () => new Date(1758000000000 + (n++) * 1000) } }); + }; + const holding = (userId: string, positions: unknown[], extra: Record = {}) => + ({ userId, tenantId: 't1', positions, permissions: [], ...extra }) as any; + /** Staffed into the position the requests are routed to; neither the submitter nor an admin. */ + const HOLDER = holding('u_holder', ['sales_manager']); + const SLOT = 'position:sales_manager'; + const toPosition = [{ type: 'position', value: 'sales_manager' }]; + /** The three override doors, each holding no slot (same org as every request here: `t1`). */ + const RESCUERS: Array<[string, any]> = [ + ['a platform admin (admin_full_access)', holding('root', [], { permissions: ['admin_full_access'] })], + ['a platform admin (PLATFORM_ADMIN posture)', holding('root2', [], { posture: 'PLATFORM_ADMIN' })], + ['a same-org tenant admin (TENANT_ADMIN posture)', holding('tadmin', [], { posture: 'TENANT_ADMIN' })], + ]; + + let recordSeq = 0; + const open = async (svc: ApprovalService, approvers: any[]) => { + const recordId = `ret_${++recordSeq}`; + const out = await svc.openNodeRequest({ + object: 'opportunity', recordId, runId: `run_ret${recordSeq}`, nodeId: 'approve_step', + flowName: 'deal_approval', + config: { approvers, behavior: 'first_response' as any }, + record: { id: recordId, amount: 100 }, + }, CTX); + if (!('id' in out)) throw new Error('scene did not open: the empty slate auto-approved'); + return out; + }; + const actionsOf = (engine: any, requestId: string, action: string) => + (engine._tables['sys_approval_action'] ?? []).filter((a: any) => a.request_id === requestId && a.action === action); + /** `can_act` as `attachViewers` serves it — for a caller the participant gate hides the request from too. */ + const servedCanAct = async (svc: ApprovalService, row: any, ctx: any) => { + const copy = { ...row }; + (svc as any).attachViewers([copy], ctx, await (svc as any).actingCaller(ctx)); + return copy.viewer.can_act as boolean; + }; + + /** The two request classes the retirement leaves to the override, each with the slot it carries. */ + const LEFT_BEHIND: Array<[string, (engine: any, svc: ApprovalService) => Promise<{ id: string; slot: string }>]> = [ + ['a slot a 15.x-era request stored as role:

', async (engine, svc) => { + const req = await open(svc, toPosition); + await storeLegacyRoleSlot(engine, svc, req.id); + return { id: req.id, slot: 'role:sales_manager' }; + }], + ['a new request from { type: role, value: } whose tier lookup finds no one', async (_engine, svc) => { + const req = await open(svc, [{ type: 'role', value: 'sales_manager' }]); + return { id: req.id, slot: 'org_membership_level:sales_manager' }; + }], + ]; + + it('a position:

slot is listed, decidable and can_act for its holder, with the default actor', async () => { + const engine = makeFakeEngine(); + const svc = svcFor(engine); + const req = await open(svc, toPosition); + expect(req.pending_approvers).toEqual([SLOT]); + + const filter = { status: 'pending' as const, approverId: ['u_holder', SLOT] }; + expect((await svc.listRequests(filter, HOLDER)).map(r => r.id)).toEqual([req.id]); + expect(await svc.countRequests(filter, HOLDER)).toBe(1); + expect((await svc.getRequest(req.id, HOLDER))?.viewer?.can_act).toBe(true); + + const out = await svc.decideNode(req.id, { decision: 'approve' } as any, HOLDER); + expect(out.finalized).toBe(true); + expect(out.request.status).toBe('approved'); + const [act] = actionsOf(engine, req.id, 'approve'); + expect([act.actor_id, act.acted_as, act.via_override]).toEqual(['u_holder', SLOT, false]); + }); + + it('role:

is no longer an address of that slot: not listed, not counted, not an actor — and the slot is untouched', async () => { + const engine = makeFakeEngine(); + const svc = svcFor(engine); + const req = await open(svc, toPosition); + + const retired = { status: 'pending' as const, approverId: ['u_holder', 'role:sales_manager'] }; + expect(await svc.listRequests(retired, HOLDER)).toEqual([]); + expect(await svc.countRequests(retired, HOLDER)).toBe(0); + // SYS sees every row, so this miss is the filter's verdict alone. + expect(await svc.listRequests({ approverId: 'role:sales_manager' }, SYS)).toEqual([]); + + await expect(svc.decideNode(req.id, { decision: 'approve', actorId: 'role:sales_manager' }, HOLDER)) + .rejects.toThrow(/^FORBIDDEN: cannot act as 'role:sales_manager'/); + const after = await svc.getRequest(req.id, SYS); + expect([after?.status, after?.pending_approvers]).toEqual(['pending', [SLOT]]); + expect(actionsOf(engine, req.id, 'approve')).toEqual([]); + }); + + describe.each(LEFT_BEHIND)('left to the override: %s', (_label, plant) => { + it('the holder of the same-named position cannot decide it: not listed, not visible, can_act false, refused', async () => { + const engine = makeFakeEngine(); + const svc = svcFor(engine); + const { id, slot } = await plant(engine, svc); + const row = await svc.getRequest(id, SYS); + expect(row?.pending_approvers).toEqual([slot]); + + for (const spelling of [SLOT, 'role:sales_manager']) { + const listed = await svc.listRequests({ status: 'pending', approverId: ['u_holder', spelling] }, HOLDER); + expect(listed, `listed under '${spelling}'`).toEqual([]); + } + expect(await svc.getRequest(id, HOLDER)).toBeNull(); + expect(await servedCanAct(svc, row, HOLDER)).toBe(false); + await expect(svc.decideNode(id, { decision: 'approve' } as any, HOLDER)) + .rejects.toThrow("FORBIDDEN: actor 'u_holder' is not a pending approver"); + await expect(svc.decideNode(id, { decision: 'approve', actorId: 'role:sales_manager' }, HOLDER)) + .rejects.toThrow(/^FORBIDDEN: cannot act as 'role:sales_manager'/); + expect(actionsOf(engine, id, 'approve')).toEqual([]); + }); + + it.each(RESCUERS)('%s decides it through decide() — by the override, and the run resumes', async (_who, admin) => { + const engine = makeFakeEngine(); + const svc = svcFor(engine); + const resumed: Array<{ runId: string; signal: any }> = []; + svc.attachAutomation({ async resume(runId: string, signal: any) { resumed.push({ runId, signal }); } }); + const { id } = await plant(engine, svc); + expect((await svc.getRequest(id, admin))?.viewer).toMatchObject({ can_act: false, can_override: true }); + + // What `POST /approvals/requests/:id/approve` calls: the actor is the caller. + const out = await svc.decide(id, { decision: 'approve', actorId: admin.userId }, admin); + expect(out.finalized).toBe(true); + expect(out.request.status).toBe('approved'); + expect(out.resumed).toBe(true); + expect(resumed).toHaveLength(1); + expect(resumed[0].signal).toMatchObject({ branchLabel: 'approve' }); + const [act] = actionsOf(engine, id, 'approve'); + expect([act.actor_id, act.acted_as, act.via_override]).toEqual([admin.userId, null, true]); + }); + + it('a reassign to the holder rescues it too: the holder then decides with the default actor', async () => { + const engine = makeFakeEngine(); + const svc = svcFor(engine); + const { id } = await plant(engine, svc); + const [, admin] = RESCUERS[2]; + const moved = await svc.reassign(id, { actorId: admin.userId, to: 'u_holder' } as any, admin); + expect(moved.request.pending_approvers).toEqual(['u_holder']); + const out = await svc.decideNode(id, { decision: 'approve' } as any, HOLDER); + expect(out.finalized).toBe(true); + expect(out.request.status).toBe('approved'); + const [act] = actionsOf(engine, id, 'approve'); + expect([act.actor_id, act.acted_as, act.via_override]).toEqual(['u_holder', 'u_holder', false]); + }); + }); +}); + // ── The person in `actor_id`, the slot in `acted_as` (#21411) ────────── // // `actor_id` is a `sys_user` lookup (ADR-0118 D1: an id or nothing), but a diff --git a/packages/plugins/plugin-approvals/src/approval-service.ts b/packages/plugins/plugin-approvals/src/approval-service.ts index 2c3ff42557d..3f5ec962d00 100644 --- a/packages/plugins/plugin-approvals/src/approval-service.ts +++ b/packages/plugins/plugin-approvals/src/approval-service.ts @@ -1598,10 +1598,11 @@ export class ApprovalService implements IApprovalService { // Named something else — allow it ONLY if the server can prove the caller // holds that identity. `positions` is resolved by the shared authz resolver - // (never client-supplied); `role:` is the ADR-0090 D3 deprecated spelling - // that 15.x-era slots and the Console's own identity list still carry. The - // spellings come from `positionAddresses` — the one equivalence every - // reader takes (`approver-address.ts`). Which SLOT the admitted actor then + // (never client-supplied), and a held position is named `position:

`: + // the address comes from `positionAddresses` — the one equivalence every + // reader takes (`approver-address.ts`). `role:

` names no position + // (ADR-0090 D3, no alias window), so it is refused here like any other + // identity the caller cannot prove. Which SLOT the admitted actor then // takes is the decision methods' question (`takenSlot`). const named = String(actorId); for (const position of context.positions ?? []) { @@ -1661,12 +1662,12 @@ export class ApprovalService implements IApprovalService { * with `actorId` exactly as {@link ApprovalService.resolveActor} returned it. * * The default actor (the caller's own user id) takes a slot under their user - * id, their account's email or any spelling of a position they hold; a named - * position address takes its position's slot under either spelling. That is - * the set `resolveActor` already admitted — a holder could always decide a + * id, their account's email or the `position:

` address of a position they + * hold; a named position address takes its position's slot. That is the set + * `resolveActor` already admitted — a holder could always decide a * `position:

` slot by naming that exact spelling — so this widens nobody: - * it makes the default actor and the console's `role:

` reach the same - * slot. A user who holds a different position takes nothing. + * it makes the default actor reach the same slot. A user who holds a + * different position takes nothing. * * ⭐ What a slot-gated action records: TWO facts, each in its own column. * `sys_approval_action.acted_as` is the slot it was admitted under — the @@ -1709,7 +1710,8 @@ export class ApprovalService implements IApprovalService { * - `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. + * (ADR-0090 D3) for one window: it resolves identically — the empty-lookup + * fallback literal included — and logs a warning. * * **Out-of-office (#1322 M1):** individually-routed approvers — the ones that * resolve to a specific person (`user` / `field` / `manager`) — are passed @@ -1798,9 +1800,11 @@ export class ApprovalService implements IApprovalService { substitutions?: OooSubstitution[], ): Promise { // 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. + // Resolve on the canonical type, and write the canonical type in the + // `type:value` fallback below too: no path writes a `role:` slot, a + // spelling no reader addresses (`approver-address.ts`). A slot stored under + // it by a 15.x-era request stays as stored, decidable by the privileged + // override — no stored slot is rewritten. const type = canonicalApproverType(String(a.type)); if (type !== a.type) { this.logger?.warn?.( @@ -1915,7 +1919,7 @@ export class ApprovalService implements IApprovalService { { type, value: a.value, organizationId: organizationId ?? null }, ); } - return [`${a.type}:${a.value}`]; + return [`${type}:${a.value}`]; } /** @@ -4548,7 +4552,7 @@ export class ApprovalService implements IApprovalService { }, { context: SYSTEM_CTX }); // Per-approver fan-out: concrete identities (user ids / emails) each get - // their OWN one-tap approve/reject links (ADR-0043); `role:*`-style + // their OWN one-tap approve/reject links (ADR-0043); `position:*`-style // literals can't carry a personal token and fall back to a plain nudge. let notified = 0; const concrete = pending.filter(a => a && !a.includes(':')); @@ -6143,7 +6147,7 @@ export class ApprovalService implements IApprovalService { } // Display names for submitters AND user-id approvers in one lookup. - // `role:` (and other `type:value` literals) are already readable. + // `position:

` (and other `type:value` literals) are already readable. const userIdentifiers: Array = []; for (const r of rows) { userIdentifiers.push(r.submitter_id); @@ -6442,12 +6446,11 @@ export class ApprovalService implements IApprovalService { * slot. Returns null when the filter is absent (callers skip the id * constraint). * - * A position address matches under EVERY spelling of that position — the - * index stores the slot as it was written (`position:

` for a slot opened - * on an unstaffed position, `role:

` for a 15.x-era one), while a client - * may ask under either. The spellings come from `equivalentApproverAddresses`, - * the same equivalence `resolveActor` admits a caller under; ⛔ never a - * second fold written here. + * A position address matches through `equivalentApproverAddresses`, the + * same equivalence `resolveActor` admits a caller under: `position:

`, the + * literal a slot opened on an unstaffed position stores, is the one spelling + * (a stored 15.x-era `role:

` row is no position address, ADR-0090 D3, and + * matches only itself). ⛔ Never a second fold written here. */ private async approverRequestIds( targets: string[], @@ -6491,12 +6494,11 @@ export class ApprovalService implements IApprovalService { * "Current approver" means a pending slot the caller could ACT under, which * is wider than their concrete user id: position/team/manager/field * approvers are resolved to concrete user ids at open time, but a position - * that nobody held at open time leaves the literal `position:

` slot (and a - * 15.x-era slot reads `role:

`), a `user` approver authored as an email - * leaves the email, and the default actor takes any of those — a position - * `p` the caller holds (server-resolved `context.positions`, never - * client-supplied) under either spelling, the email their own account - * carries. Keying on the user id alone hid exactly those requests from the + * that nobody held at open time leaves the literal `position:

` slot, a + * `user` approver authored as an email leaves the email, and the default + * actor takes any of those — a position `p` the caller holds + * (server-resolved `context.positions`, never client-supplied) as + * `position:

`, the email their own account carries. Keying on the user id alone hid exactly those requests from the * people who could decide them: absent from "My Pending" under every * spelling the client asked for, `404` on the request itself. So the probe * asks for the caller's acting addresses (`actingAddresses`, diff --git a/packages/plugins/plugin-approvals/src/approver-address-readers.test.ts b/packages/plugins/plugin-approvals/src/approver-address-readers.test.ts index 6a811054c6a..6a17ea2913e 100644 --- a/packages/plugins/plugin-approvals/src/approver-address-readers.test.ts +++ b/packages/plugins/plugin-approvals/src/approver-address-readers.test.ts @@ -49,6 +49,26 @@ * in one line: slots are compared only with `acted_as`, and `actor_id` only * with a caller's user id. * + * 4. NO `role:` SLOT (ADR-0090 D3). `position:

` is the one spelling of a + * position address: no writer in this package produces a `role:` slot, and + * no reader compares one. Two scans over EVERY non-test `.ts` under `src/`, + * `approver-address.ts` included: + * - THE LITERALS. Every string-like literal — a quoted string, a + * no-substitution template, each literal piece of a template — is read + * from the syntax tree, so a comment is masked by construction. None + * may carry a `role:` prefix: that is a reader's prefix (`'role:'` back + * in the equivalence), a compared `` `role:${p}` ``, and a writer's + * literal alike. Positive control: the same scan finds the `position:` + * prefix in `approver-address.ts`, so it reads the module it guards. + * - THE `:` TEMPLATES. The writer the literal scan cannot + * see is an interpolated TYPE: `resolveApproverSpec`'s fallback once + * wrote `${a.type}:${a.value}`, the AUTHORED spelling, so a flow still + * authoring the deprecated `role` approver type opened `role:` slots. + * Every template of that shape is classified in `TYPE_VALUE_TEMPLATES` + * (new or changed ones fail with their location, as in 2), and none + * may interpolate a `.type` property: the fallback's `type` is the + * `canonicalApproverType(...)` result, pinned by name. + * * What it does not see: a comparison spelled with none of those shapes (a * hand-written loop over a slate with `===`). The shapes are the ones every * reader in this file's history has used; a new spelling is caught in review, @@ -180,6 +200,86 @@ const PERSON_SITES: Readonly:` + * literal in the making — keyed ` · · `. A slot writer's type must be one no flow can spell `role`. + */ +const TYPE_VALUE_TEMPLATES: Readonly> = { + 'approval-service.ts · resolveApproverSpec · `${type}:${a.value}`': { + role: 'slot-writer', + why: 'the empty-lookup fallback; `type` is canonicalApproverType(a.type), so the deprecated `role` type writes ' + + '`org_membership_level:` (ADR-0090 D3)', + }, + 'approval-service.ts · resolveExpressionApprovers · `${resolveAs}:${key}`': { + role: 'slot-writer', + why: 'an expression approver\'s empty expansion; `resolveAs` is one of department / position / team here — any ' + + 'other value threw VALIDATION_FAILED above', + }, + 'approval-service.ts · expandApprovers · `${groupKey}:${entry.subGroup}`': { + role: 'not-a-slot', + why: 'a per_group group key (#3266), never written to a slate', + }, +}; + +/** Every string-like literal piece in `sf`: its cooked text and its node. */ +function literalPieces(sf: ts.SourceFile): Array<{ text: string; node: ts.Node }> { + const out: Array<{ text: string; node: ts.Node }> = []; + const visit = (node: ts.Node): void => { + if (ts.isStringLiteral(node) || ts.isNoSubstitutionTemplateLiteral(node)) out.push({ text: node.text, node }); + if (ts.isTemplateExpression(node)) { + out.push({ text: node.head.text, node }); + for (const span of node.templateSpans) out.push({ text: span.literal.text, node }); + } + ts.forEachChild(node, visit); + }; + visit(sf); + return out; +} + +/** + * Every template in `sf` that opens with `${X}:` and no space after the colon — + * the `:` shape (`${path}: ${message}` is prose, not an address). + */ +function typeValueTemplates(sf: ts.SourceFile): ts.TemplateExpression[] { + const out: ts.TemplateExpression[] = []; + const visit = (node: ts.Node): void => { + if (ts.isTemplateExpression(node) && node.head.text === '' && /^:(?!\s)/.test(node.templateSpans[0]?.literal.text ?? '')) { + out.push(node); + } + ts.forEachChild(node, visit); + }; + visit(sf); + return out; +} + +/** Is the template's type interpolation an AUTHORED spelling — a `.type` property read? */ +function interpolatesAuthoredType(template: ts.TemplateExpression): boolean { + const first = template.templateSpans[0].expression; + return ts.isPropertyAccessExpression(first) && first.name.text === 'type'; +} + +/** Every non-test `.ts` under `src/`, the equivalence module included. */ +function allSourceFiles(dir: string): string[] { + const out: string[] = []; + for (const name of readdirSync(dir)) { + const path = join(dir, name); + if (statSync(path).isDirectory()) out.push(...allSourceFiles(path)); + else if (name.endsWith('.ts') && !name.endsWith('.test.ts')) out.push(path); + } + return out; +} + function sourceFiles(dir: string): string[] { const out: string[] = []; for (const name of readdirSync(dir)) { @@ -337,6 +437,75 @@ describe('approver-address — the readers of the equivalence, and no comparison expect(method(service, 'visibleRequestIds').getText(service)).toMatch(/const uid = who\.userId;/); }); + it('no role: slot (ADR-0090 D3): no string literal in this package carries the retired prefix — comments masked, position: found as the positive control', () => { + const files = allSourceFiles(HERE); + expect(files.map((f) => relative(HERE, f))).toContain(EQUIVALENCE_MODULE); + const retired: string[] = []; + const live: string[] = []; + for (const path of files) { + const sf = parse(path); + for (const { text, node } of literalPieces(sf)) { + const where = `${relative(HERE, path)}:${sf.getLineAndCharacterOfPosition(node.getStart(sf)).line + 1} · ${JSON.stringify(text)}`; + if (RETIRED_PREFIX.test(text)) retired.push(where); + if (LIVE_PREFIX.test(text)) live.push(where); + } + } + expect( + retired, + 'a `role:` slot literal: position:

is the one position address (ADR-0090 D3) — write position:, and never ' + + 'put role: back into approver-address.ts', + ).toEqual([]); + // The positive control: the scan reads the equivalence module's own prefix. + expect(live.filter((w) => w.startsWith(`${EQUIVALENCE_MODULE}:`)).length).toBeGreaterThan(0); + }); + + it('no role: slot (ADR-0090 D3): every : template is classified, and no slot writer interpolates an authored type', () => { + const sites: string[] = []; + const authored: string[] = []; + for (const path of allSourceFiles(HERE)) { + const sf = parse(path); + for (const template of typeValueTemplates(sf)) { + const key = `${relative(HERE, path)} · ${enclosingFunction(template, sf)} · ${template.getText(sf).replace(/\s+/g, ' ')}`; + sites.push(key); + if (interpolatesAuthoredType(template)) authored.push(key); + } + } + const unclassified = sites.filter((s) => !(s in TYPE_VALUE_TEMPLATES)); + expect( + unclassified, + 'a : literal is built outside the classified set: if it writes a slot, its type must be canonical ' + + '(canonicalApproverType) — classify it in TYPE_VALUE_TEMPLATES with its reason', + ).toEqual([]); + const stale = Object.keys(TYPE_VALUE_TEMPLATES).filter((s) => !sites.includes(s)); + expect(stale, 'a classified template changed or disappeared: re-classify it').toEqual([]); + expect( + authored, + 'a template interpolates `.type` — the AUTHORED approver type, which a flow can still spell `role`: ' + + 'interpolate the canonicalApproverType(...) result instead', + ).toEqual([]); + // The fallback writer's `type` IS the canonical type, by name. + const service = parse(join(HERE, SERVICE)); + expect(method(service, 'resolveApproverSpec').getText(service)) + .toMatch(/const type = canonicalApproverType\(String\(a\.type\)\);[\s\S]*return \[`\$\{type\}:\$\{a\.value\}`\];/); + }); + + it('the role: scans catch what they name (planted literals, comments and templates)', () => { + const planted = ts.createSourceFile('planted.ts', [ + '// role:sales_manager in a comment is masked', + '/* role:x */ const prefixes = [\'position:\', \'role:\'];', + 'const asked = (p: string) => `role:${p}`;', + 'const csv = \'u1,role:finance\';', + 'const column = \'sys_member.role: owner\';', + 'const key = { role: \'admin\' };', + 'function f(a: any, type: string) { return [`${a.type}:${a.value}`, `${type}:${a.value}`, `${a.path}: ${a.message}`]; }', + ].join('\n'), ts.ScriptTarget.Latest, true); + const flagged = literalPieces(planted).filter(({ text }) => RETIRED_PREFIX.test(text)).map(({ text }) => text); + expect(flagged).toEqual(['role:', 'role:', 'u1,role:finance']); + const templates = typeValueTemplates(planted); + expect(templates.map((t) => t.getText(planted))).toEqual(['`${a.type}:${a.value}`', '`${type}:${a.value}`']); + expect(templates.map(interpolatesAuthoredType)).toEqual([true, false]); + }); + it('the detector catches the shapes it names (a planted user-id comparison in each shape)', () => { const planted = ts.createSourceFile('planted.ts', [ 'class S {', diff --git a/packages/plugins/plugin-approvals/src/approver-address.test.ts b/packages/plugins/plugin-approvals/src/approver-address.test.ts index c555980bb9b..8cf08cccbd4 100644 --- a/packages/plugins/plugin-approvals/src/approver-address.test.ts +++ b/packages/plugins/plugin-approvals/src/approver-address.test.ts @@ -5,9 +5,10 @@ * * Every reader that compares a slot address with a caller's identity takes it * from here (`approver-address-readers.test.ts` enumerates them), so these - * pins are on the equivalence itself: exactly the two spellings the acting - * path has always admitted, nothing else folds, and the default actor acts - * under exactly the caller's server-resolved addresses. The service-level pins + * pins are on the equivalence itself: exactly the one spelling the acting + * path admits (`position:

` — ADR-0090 D3 retired `role:

` with no alias + * window), nothing else folds, and the default actor acts under exactly the + * caller's server-resolved addresses. The service-level pins * (one per reader) live beside the other participant-visibility pins in * `approval-service.test.ts`. */ @@ -22,19 +23,18 @@ import { } from './approver-address.js'; describe('approver-address — the position-address equivalence', () => { - it('spells a position under exactly the two prefixes the acting path admits, canonical first', () => { - expect(positionAddresses('sales_manager')).toEqual(['position:sales_manager', 'role:sales_manager']); + it('spells a position under exactly the one prefix the acting path admits: position:', () => { + expect(positionAddresses('sales_manager')).toEqual(['position:sales_manager']); }); - it('folds either spelling of a position onto both', () => { - const both = ['position:sales_manager', 'role:sales_manager']; - expect(equivalentApproverAddresses('position:sales_manager')).toEqual(both); - expect(equivalentApproverAddresses('role:sales_manager')).toEqual(both); + it('a position: address names its position; the retired role: spelling is no longer an address of it (ADR-0090 D3)', () => { + expect(equivalentApproverAddresses('position:sales_manager')).toEqual(['position:sales_manager']); + expect(equivalentApproverAddresses('role:sales_manager')).toEqual(['role:sales_manager']); }); - it('splits on the FIRST accepted prefix only, so a position name may itself contain a colon', () => { - expect(equivalentApproverAddresses('role:position:x')).toEqual(['position:position:x', 'role:position:x']); - expect(equivalentApproverAddresses('position:role:x')).toEqual(['position:role:x', 'role:role:x']); + it('splits on the accepted prefix at the start only, so a position name may itself contain a colon', () => { + expect(equivalentApproverAddresses('role:position:x')).toEqual(['role:position:x']); + expect(equivalentApproverAddresses('position:role:x')).toEqual(['position:role:x']); }); // The negative control: a fold wider than the acting path would list @@ -42,6 +42,7 @@ describe('approver-address — the position-address equivalence', () => { it.each([ ['a user id', 'u_reviewer'], ['an email', 'reviewer@example.com'], + ['the retired role: spelling (ADR-0090 D3)', 'role:sales_manager'], ['a team literal', 'team:sales_manager'], ['a membership-level literal', 'org_membership_level:sales_manager'], ['a department literal', 'department:sales_manager'], @@ -56,19 +57,19 @@ describe('approver-address — the position-address equivalence', () => { describe('approver-address — the caller\'s acting addresses and the one slot test (#21379)', () => { const caller = { userId: 'u_holder', email: 'holder@example.com', positions: ['sales_manager', 'finance'] }; - it('the default actor acts under the user id, the account email and both spellings of each held position, in that order', () => { + it('the default actor acts under the user id, the account email and the position: address of each held position, in that order', () => { expect(actingAddresses(caller)).toEqual([ 'u_holder', 'holder@example.com', - 'position:sales_manager', 'role:sales_manager', 'position:finance', 'role:finance', + 'position:sales_manager', 'position:finance', ]); expect(actingAddresses({ userId: 'u_bare' })).toEqual(['u_bare']); expect(actingAddresses({ userId: 'u_bare', email: null, positions: [] })).toEqual(['u_bare']); }); - it('a named actor acts under itself — plus the other spelling when it names a position — never under the caller\'s other addresses', () => { + it('a named actor acts under itself, never under the caller\'s other addresses — and role: folds onto no position', () => { expect(actorAddresses('u_holder', caller)).toEqual(actingAddresses(caller)); - expect(actorAddresses('role:sales_manager', caller)).toEqual(['role:sales_manager', 'position:sales_manager']); - expect(actorAddresses('position:sales_manager', caller)).toEqual(['position:sales_manager', 'role:sales_manager']); + expect(actorAddresses('role:sales_manager', caller)).toEqual(['role:sales_manager']); + expect(actorAddresses('position:sales_manager', caller)).toEqual(['position:sales_manager']); expect(actorAddresses('holder@example.com', caller)).toEqual(['holder@example.com']); // A machine caller (no resolved caller) keeps its minted actor literally. expect(actorAddresses('system:sla', null)).toEqual(['system:sla']); @@ -76,10 +77,11 @@ describe('approver-address — the caller\'s acting addresses and the one slot t it('heldSlot takes the first acting address the slate holds, and nothing a different position spells', () => { expect(heldSlot(['position:sales_manager'], 'u_holder', caller)).toBe('position:sales_manager'); - expect(heldSlot(['role:sales_manager'], 'u_holder', caller)).toBe('role:sales_manager'); + // A stored role: slot is not the position's: its holder takes nothing there. + expect(heldSlot(['role:sales_manager'], 'u_holder', caller)).toBeUndefined(); expect(heldSlot(['holder@example.com'], 'u_holder', caller)).toBe('holder@example.com'); expect(heldSlot(['position:sales_manager', 'u_holder'], 'u_holder', caller)).toBe('u_holder'); - expect(heldSlot(['position:sales_manager'], 'role:sales_manager', caller)).toBe('position:sales_manager'); + expect(heldSlot(['position:sales_manager'], 'role:sales_manager', caller)).toBeUndefined(); // Negative controls: another position, another person, a non-position literal. expect(heldSlot(['position:cfo'], 'u_holder', caller)).toBeUndefined(); expect(heldSlot(['u_other', 'other@example.com'], 'u_holder', caller)).toBeUndefined(); diff --git a/packages/plugins/plugin-approvals/src/approver-address.ts b/packages/plugins/plugin-approvals/src/approver-address.ts index 2d9cf175c3d..c5691d3dd64 100644 --- a/packages/plugins/plugin-approvals/src/approver-address.ts +++ b/packages/plugins/plugin-approvals/src/approver-address.ts @@ -6,21 +6,29 @@ * applies wherever a slot address is compared with a caller's identity. * * A pending slot that routed to a position nobody held at open time stores the - * literal `position:

` (`resolveApproverSpec`'s `type:value` fallback); a - * 15.x-era slot, and the Console's own identity list, spell the same position - * `role:

` — the ADR-0090 D3 deprecated spelling. A `user` approver authored - * as an email stores the email. `resolveActor` lets a caller act under each of - * those — a position `p` they hold (server-resolved `context.positions`, never - * client-supplied) under either spelling, an email their own account carries — - * as well as under their bare user id. + * literal `position:

` (`resolveApproverSpec`'s `type:value` fallback). A + * `user` approver authored as an email stores the email. `resolveActor` lets a + * caller act under each of those — a position `p` they hold (server-resolved + * `context.positions`, never client-supplied) as `position:

`, an email + * their own account carries — as well as under their bare user id. + * + * `position:

` is the ONE spelling of a position address. `role:

` is not + * one: ADR-0090 D3 retires the word `role` with no alias window, so a slot + * stored under that spelling — a 15.x-era request, or one the deprecated + * `role` approver type opened before its fallback literal was canonicalized — + * addresses no position holder. Only the privileged override decides it, or a + * reassign to a real approver. Nothing in this package writes the spelling any + * more (`resolveApproverSpec` writes the canonical type), and + * `approver-address-readers.test.ts` fails on a `role:` slot literal written + * anywhere in its source. * * The readers used to disagree with it, one at a time. The inbox's "My * Pending" filter matched the caller's `approverId` values literally; the * participant gate, the per-viewer `can_act` flag, the decision methods' slot * test and the already-acted probe each keyed on the bare user id. So a holder * of `p` could decide a `position:

` request only by naming that exact - * spelling — the default actor and the console's `role:

` were refused, the - * console hid its decision buttons, and the request vanished from the holder's + * spelling — the default actor was refused, the console hid its decision + * buttons, and the request vanished from the holder's * sight the moment they decided it. Readers of one equivalence, written * separately, had drifted. * @@ -28,8 +36,8 @@ * * - `resolveActor` — may this caller act under the named address? * ({@link positionAddresses}) - * - `approverRequestIds` — which requests hold a slot under any spelling of - * the addresses the caller asked about? ({@link equivalentApproverAddresses}) + * - `approverRequestIds` — which requests hold a slot under the addresses the + * caller asked about? ({@link equivalentApproverAddresses}) * - `visibleRequestIds`, current approver — which requests hold a slot the * caller acts under? ({@link actingAddresses}) * - `visibleRequestIds`, already acted — which requests carry a decision or @@ -41,20 +49,22 @@ * * ⛔ Do not write a second fold beside any of them, and do not widen this one * to a spelling the acting path does not accept: the set below IS the acting - * path's set. A reader that folds more than the acting path accepts would show + * path's set. ⛔ Nor put `role:` back into it — that reopens the alias window + * ADR-0090 D3 closed. A reader that folds more than the acting path accepts would show * a user requests they cannot decide; one that folds less hides requests they * can. `approver-address-readers.test.ts` enumerates the readers and fails on * a slot-against-caller comparison written anywhere else. */ /** - * The prefixes under which a slot address names a position. The order is the - * order {@link positionAddresses} spells them in, canonical first. + * The prefixes under which a slot address names a position: `position:` alone + * (ADR-0090 D3 — the retired `role:` is no alias of it). Kept a list so every + * reader below reads the one set the acting path admits. */ -const POSITION_ADDRESS_PREFIXES = ['position:', 'role:'] as const; +const POSITION_ADDRESS_PREFIXES = ['position:'] as const; /** - * Every slot address that names `position`, canonical spelling first. + * Every slot address that names `position` — `position:`. * * Interpolated exactly as the acting path always spelled it, so a non-string * entry in `context.positions` produces the same strings it always did. @@ -65,10 +75,10 @@ export function positionAddresses(position: unknown): string[] { /** * Every slot address the approval service treats as the SAME identity as - * `address`: the address itself, plus — when it names a position under any - * accepted prefix — every other spelling of that position. An address that + * `address`: when it names a position, that position's addresses + * ({@link positionAddresses}); otherwise the address alone. An address that * names no position (a user id, an email, a `team:` / `org_membership_level:` - * literal) is equivalent only to itself. + * literal, a stored `role:` slot) is equivalent only to itself. */ export function equivalentApproverAddresses(address: string): string[] { for (const prefix of POSITION_ADDRESS_PREFIXES) { @@ -97,7 +107,7 @@ export interface ActingCaller { /** * Every slot address the caller acts under WITHOUT naming one — the default * actor — in the order a slot is taken: the user id, the account's email, then - * every spelling of every held position. + * the address of every held position. * * The email is matched as the account stores it. `resolveActor` admits a * NAMED email case-insensitively, and that named spelling is judged by @@ -116,8 +126,7 @@ export function actingAddresses(caller: ActingCaller): string[] { * * - the caller's own user id — the default actor, and what the REST routes * pass when the body names nobody → {@link actingAddresses}; - * - a position address → that spelling first, then every other spelling of - * the same position; + * - a position address → that position's addresses; * - anything else (a named email; a machine caller's minted actor) → itself. */ export function actorAddresses(actorId: string, caller: ActingCaller | null): string[] { diff --git a/packages/plugins/plugin-approvals/src/sys-approval-approver.object.ts b/packages/plugins/plugin-approvals/src/sys-approval-approver.object.ts index 839d1fad216..7a0ff7e5140 100644 --- a/packages/plugins/plugin-approvals/src/sys-approval-approver.object.ts +++ b/packages/plugins/plugin-approvals/src/sys-approval-approver.object.ts @@ -19,7 +19,7 @@ import { F } from '@objectstack/spec'; * open approvals, not the append-only request history. * * `approver` holds one identity literal exactly as it appears in the CSV: - * a user id, an email, or a `role:` / `team:` style literal. + * a user id, an email, or a `position:` / `team:` style literal. * Equality (or `$in`) on this column is the indexed replacement for the old * per-row substring match. * @@ -75,7 +75,7 @@ export const SysApprovalApprover = ObjectSchema.create({ label: 'Approver', required: true, maxLength: 255, - description: 'One pending-approver identity: user id, email, or role:/team: literal', + description: 'One pending-approver identity: user id, email, or position:/team: literal', group: 'Target', }), diff --git a/packages/qa/dogfood/test/fixtures/my-pending-position-fixture.ts b/packages/qa/dogfood/test/fixtures/my-pending-position-fixture.ts index 047f8d95afa..18550b6b071 100644 --- a/packages/qa/dogfood/test/fixtures/my-pending-position-fixture.ts +++ b/packages/qa/dogfood/test/fixtures/my-pending-position-fixture.ts @@ -7,7 +7,8 @@ // is the literal `position:

` slot, exactly what a HotCRM approval carries // when its approver position is staffed only after submission. The pin then // staffs a user into that position and asks the approvals inbox's "My -// Pending" door for the request under both approver-address spellings. +// Pending" door for the request: `position:

` lists it, and the retired +// `role:

` spelling (ADR-0090 D3) does not. // // Purpose-built rather than borrowed from an example app for the reason // `override-composite-fixture.ts` gives: showcase's `onEnable` staffs its own diff --git a/packages/qa/dogfood/test/my-pending-position-address.dogfood.test.ts b/packages/qa/dogfood/test/my-pending-position-address.dogfood.test.ts index 2ba8b755a1e..61c8385bf33 100644 --- a/packages/qa/dogfood/test/my-pending-position-address.dogfood.test.ts +++ b/packages/qa/dogfood/test/my-pending-position-address.dogfood.test.ts @@ -3,10 +3,10 @@ // #21350 — "My Pending" never listed a request routed to a position. // // A request whose approver position nobody holds when it opens keeps the -// literal `position:

` slot. Staff a user into `p` afterwards and they can -// approve it — `resolveActor` admits a holder under `position:

` or -// `role:

` — yet the approvals inbox's "My Pending" door never showed it: -// the console asks under `role:

`, and the list filter matched literally. +// literal `position:

` slot. Staff a user into `p` afterwards and they could +// approve it — `resolveActor` admitted a holder under `position:

` (and, +// then, `role:

`) — yet the approvals inbox's "My Pending" door never showed +// it: the console asked under `role:

`, and the list filter matched literally. // Measured on a real boot before the fix, the defect had TWO halves: // // - the STAFFED SUBMITTER (HotCRM's shape) found it under `position:

` @@ -17,14 +17,18 @@ // a "current approver" on the bare user id). // // Both now read the acting path's ONE position-address equivalence -// (`plugin-approvals/src/approver-address.ts`). This pins the door the inbox +// (`plugin-approvals/src/approver-address.ts`), whose one position spelling is +// `position:

`: ADR-0090 D3 retired `role:

` with no alias window, and the +// pinned console now asks under `position:

`. This pins the door the inbox // actually calls, on a booted app, with both halves and both controls: // -// ⭐ either spelling lists the request, for the reviewer AND the submitter; +// ⭐ `position:

` lists the request, for the reviewer AND the submitter; +// the retired `role:

` lists nothing for either; // ⭐ a user who does not hold the position sees nothing (negative control); -// ⭐ a spelling the acting path does not admit folds onto nothing; -// ⭐ the acting path is unchanged — the reviewer decides under the stored -// spelling, the bystander is refused (the control). +// ⭐ a spelling the acting path does not admit — `role:` among them — folds +// onto nothing; +// ⭐ the acting path — the reviewer decides under the stored spelling, the +// bystander is refused, and the reviewer naming `role:

` is refused. import { describe, it, expect } from 'vitest'; import { bootStack } from '@objectstack/verify'; @@ -46,9 +50,9 @@ interface ListBody { data: Array<{ id: string; pending_approvers?: string[] }>; } -describe('"My Pending" lists a position-routed request under either spelling (#21350)', () => { +describe('"My Pending" lists a position-routed request under position: (#21350; role: retired, ADR-0090 D3)', () => { it( - 'a holder of the routed position finds it under role: and position:, a non-holder does not, and the acting path is unchanged', + 'a holder of the routed position finds it under position: and not under role:, a non-holder does not, and the acting path refuses role:', async () => { const stack = await bootStack(myPendingStack as unknown as Parameters[0], { automation: true, @@ -94,15 +98,19 @@ describe('"My Pending" lists a position-routed request under either spelling (#2 expect(all[0].pending_approvers).toEqual([`position:${ROUTED_POSITION}`]); const requestId = all[0].id; - // ⭐ Either spelling, for the reviewer (the participant gate's half) + // ⭐ `position:

`, for the reviewer (the participant gate's half) // and for the submitter (the filter's half) — the console's identity - // list is `,,role:

`. - for (const spelling of [`role:${ROUTED_POSITION}`, `position:${ROUTED_POSITION}`]) { - const asReviewer = await myPending(reviewerToken, [reviewerId, REVIEWER, spelling]); - expect(asReviewer.map((r) => r.id), `reviewer under '${spelling}'`).toEqual([requestId]); - const asSubmitter = await myPending(submitterToken, [submitterId, SUBMITTER, spelling]); - expect(asSubmitter.map((r) => r.id), `submitter under '${spelling}'`).toEqual([requestId]); - } + // list is `,,position:

`. + const spelling = `position:${ROUTED_POSITION}`; + const asReviewer = await myPending(reviewerToken, [reviewerId, REVIEWER, spelling]); + expect(asReviewer.map((r) => r.id), `reviewer under '${spelling}'`).toEqual([requestId]); + const asSubmitter = await myPending(submitterToken, [submitterId, SUBMITTER, spelling]); + expect(asSubmitter.map((r) => r.id), `submitter under '${spelling}'`).toEqual([requestId]); + // ⭐ The retired `role:

` is no address of the slot (ADR-0090 D3): a + // filter miss — 200 with no rows — for both of them. + const retired = `role:${ROUTED_POSITION}`; + expect(await myPending(reviewerToken, [reviewerId, REVIEWER, retired]), `reviewer under '${retired}'`).toEqual([]); + expect(await myPending(submitterToken, [submitterId, SUBMITTER, retired]), `submitter under '${retired}'`).toEqual([]); const detail = await stack.apiAs(reviewerToken, 'GET', `/approvals/requests/${requestId}`); expect(detail.status).toBe(200); @@ -114,19 +122,24 @@ describe('"My Pending" lists a position-routed request under either spelling (#2 const bystanderDetail = await stack.apiAs(bystanderToken, 'GET', `/approvals/requests/${requestId}`); expect(bystanderDetail.status).toBe(404); - // ⭐ Negative control — only the two spellings the acting path admits - // fold; the admin sees every row, so a miss is the filter's verdict. - for (const address of [`team:${ROUTED_POSITION}`, `org_membership_level:${ROUTED_POSITION}`]) { + // ⭐ Negative control — only the one spelling the acting path admits + // folds; the admin sees every row, so a miss is the filter's verdict. + for (const address of [`role:${ROUTED_POSITION}`, `team:${ROUTED_POSITION}`, `org_membership_level:${ROUTED_POSITION}`]) { expect(await myPending(adminToken, [address]), `'${address}' must not fold`).toEqual([]); } - // ⭐ The acting path, unchanged (the control): a non-holder naming the - // slot is refused; the holder decides under the stored spelling. + // ⭐ The acting path (the control): a non-holder naming the slot is + // refused; the holder naming the retired spelling is refused too; the + // holder decides under the stored spelling. const refused = await stack.apiAs(bystanderToken, 'POST', `/approvals/requests/${requestId}/approve`, { actorId: `position:${ROUTED_POSITION}`, }); expect(refused.status).toBe(403); expect(((await refused.json()) as { code?: string }).code).toBe('FORBIDDEN'); + const viaRetired = await stack.apiAs(reviewerToken, 'POST', `/approvals/requests/${requestId}/approve`, { + actorId: `role:${ROUTED_POSITION}`, + }); + expect([viaRetired.status, ((await viaRetired.json()) as { code?: string }).code]).toEqual([403, 'FORBIDDEN']); const decided = await stack.apiAs(reviewerToken, 'POST', `/approvals/requests/${requestId}/approve`, { actorId: `position:${ROUTED_POSITION}`, diff --git a/packages/qa/dogfood/test/position-address-readers.dogfood.test.ts b/packages/qa/dogfood/test/position-address-readers.dogfood.test.ts index d692ca2b5ca..a153835ab3c 100644 --- a/packages/qa/dogfood/test/position-address-readers.dogfood.test.ts +++ b/packages/qa/dogfood/test/position-address-readers.dogfood.test.ts @@ -25,9 +25,12 @@ // ⭐ can_act is the default actor's decision answer, row for row — holder, // bystander, submitter, admin; // ⭐ the holder decides with the default actor AND with the console's -// spelling; the decision records the holder in `actor_id` and the slot's -// stored spelling in `acted_as` (#21411: the person and the slot are two -// facts, in two columns); +// spelling, `position:

`; the decision records the holder in +// `actor_id` and the slot's stored spelling in `acted_as` (#21411: the +// person and the slot are two facts, in two columns); +// ⭐ the retired `role:

` (ADR-0090 D3, no alias window) is no address of +// the slot: it lists nothing, and an approve naming it is refused 403 +// FORBIDDEN with nothing recorded; // ⭐ the holder keeps sight of the request after deciding it; the bystander // never sees it (this widens nobody); // ⭐ the email-keyed slot is listed, flagged, decided and kept in sight. @@ -119,7 +122,7 @@ describe('every slot reader takes the caller\'s acting addresses (#21379)', () = await ql.insert('sys_user_position', { id: 'pa_hold_bystander', user_id: bystanderId, position: OTHER_POSITION }, SYS); // ⭐ can_act, served, for every caller who may read the request… - const holderRow = (await list(holderToken, [holderId, HOLDER, `role:${ROUTED_POSITION}`])) + const holderRow = (await list(holderToken, [holderId, HOLDER, SLOT])) .find((r) => r.id === tableRequest); const submitterRow = (await list(submitterToken)).find((r) => r.id === tableRequest); const adminRow = (await list(adminToken, [SLOT])).find((r) => r.id === tableRequest); @@ -127,7 +130,9 @@ describe('every slot reader takes the caller\'s acting addresses (#21379)', () = expect(submitterRow?.viewer).toMatchObject({ can_act: false, can_override: false }); expect(adminRow?.viewer).toMatchObject({ can_act: false, can_override: true }); // …and the bystander may not read it at all. - expect(await list(bystanderToken, [bystanderId, BYSTANDER, `role:${ROUTED_POSITION}`])).toEqual([]); + expect(await list(bystanderToken, [bystanderId, BYSTANDER, SLOT])).toEqual([]); + // ⭐ The retired spelling lists nothing, for the holder either. + expect(await list(holderToken, [holderId, HOLDER, `role:${ROUTED_POSITION}`])).toEqual([]); expect(await detail(bystanderToken, tableRequest)).toBe(404); // ⭐ …is the default actor's decision answer, row for row. @@ -149,9 +154,13 @@ describe('every slot reader takes the caller\'s acting addresses (#21379)', () = expect(await detail(holderToken, tableRequest)).toBe(200); expect(await detail(bystanderToken, tableRequest)).toBe(404); - // ⭐ The console's spelling reaches the same slot. + // ⭐ The retired `role:

` is refused, and records nothing… const viaRole = await approve(holderToken, consoleSpellingRequest, { actorId: `role:${ROUTED_POSITION}` }); - expect([viaRole.status, viaRole.requestStatus]).toEqual([200, 'approved']); + expect([viaRole.status, viaRole.code]).toEqual([403, 'FORBIDDEN']); + expect(await recorded(consoleSpellingRequest)).toEqual([]); + // …while the console's spelling, `position:

`, reaches the slot. + const viaPosition = await approve(holderToken, consoleSpellingRequest, { actorId: SLOT }); + expect([viaPosition.status, viaPosition.requestStatus]).toEqual([200, 'approved']); expect(await recorded(consoleSpellingRequest)).toEqual([{ actor_id: holderId, acted_as: SLOT, via_override: false }]); // ⭐ The email-keyed slot, for a reviewer who did not submit it. diff --git a/packages/spec/src/migrations/entries/semantic/18.approval-position-address-role-retired.ts b/packages/spec/src/migrations/entries/semantic/18.approval-position-address-role-retired.ts new file mode 100644 index 00000000000..e3840cb7afd --- /dev/null +++ b/packages/spec/src/migrations/entries/semantic/18.approval-position-address-role-retired.ts @@ -0,0 +1,63 @@ +// Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license. + +import type { SemanticMigration } from '../../types.js'; + +export const entry: SemanticMigration = { + id: 'approval-position-address-role-retired', + surface: + 'approvals position address role: — the approverId filter of the approvals ' + + 'request list, the actorId of every decision, and a stored pending_approvers slot', + replacement: + '`position:`, the one spelling of a position address; a flow approver authored ' + + 'as `{ type: \'role\', value: }` becomes ' + + '`{ type: \'position\', value: }`', + reason: + 'The fourth face of the ADR-0090 D3 `role` retirement, beside ' + + '`actor-user-roles-to-positions` and `action-session-roles-to-positions`, and like them ' + + 'a runtime face with no spec schema. The approvals service read `role:` as a ' + + 'second spelling of `position:` wherever it compares a slot with the caller ' + + '(the "My Pending" filter, the participant gate, `viewer.can_act`, and the slot test of ' + + 'every decision), because 15.x-era slots and the stock console\'s identity list carried ' + + 'it. ADR-0090 D3 retires the word with no alias window, so once the pinned console sent ' + + '`position:` the arm came out in one edit (maintainer ruling, 2026-10-04). ' + + '`position:` is now the only position address: a `role:` ask matches ' + + 'only a slot stored under that exact spelling, and a `role:` actor is refused ' + + 'with 403 `FORBIDDEN`. ' + + 'The same ruling closed the one WRITER of the spelling. The deprecated `role` approver ' + + 'TYPE already resolved as `org_membership_level` (the org-membership tier: owner, admin, ' + + 'member), but when that lookup found no one the fallback slot kept the AUTHORED spelling, ' + + '`role:`, and a holder of a same-named position decided it through the arm, so ' + + 'the runtime silently honoured a membership-tier declaration as a position. The fallback ' + + 'now writes the canonical `org_membership_level:`, and no path writes a `role:` ' + + 'slot. ' + + 'Two classes of pending request are therefore decided only by the privileged override, ' + + 'or by a reassign to a real approver: a request a 15.x-era release stored as ' + + '`role:`, and a new request from a flow that still authors ' + + '`{ type: \'role\', value: }` and whose tier lookup finds no one. No ' + + 'stored slot is rewritten: the ruling refused a one-time rewrite as the permanent ' + + 'migration debt ADR-0090\'s first forcing fact names. ' + + 'Why this is a D3 semantic TODO and not a D2 conversion, on two independent grounds. ' + + 'FIRST, no metadata key moves: the address is runtime DATA (a request slot, a query ' + + 'parameter, a decision body\'s actor), never a `sys_metadata` row, so there is no source ' + + 'for a declarative transform to rewrite. SECOND, the one authored shape that leads here, ' + + '`{ type: \'role\', value: }`, is ambiguous by construction: the deprecated alias ' + + 'means the membership TIER, and whether its author meant a position instead is a ' + + 'judgment only that author can make, so a mechanical rewrite to either type would guess. ' + + 'The `ApproverType` `role` alias itself is a separate retirement and is unchanged here. ' + + 'ADR-0090 D3, ADR-0087.', + acceptanceCriteria: + 'No client sends `role:` as an `approverId` or an `actorId`: every such value is ' + + '`position:`, and a request routed to the position is listed for its holder, ' + + 'served with `viewer.can_act: true`, and decided by that holder with no `actorId` named. ' + + 'Every flow approver authored as `{ type: \'role\', value: }` is reviewed by its ' + + 'author: `` a position name becomes `{ type: \'position\', value: }`, and `` a ' + + 'membership tier (owner, admin, member) becomes ' + + '`{ type: \'org_membership_level\', value: }`. `os lint` reports the first as ' + + '`approval-approver-not-membership-tier` and the second as ' + + '`approval-approver-type-deprecated`. Every pending request whose slot reads ' + + '`role:`, or `org_membership_level:` from such a flow, is ' + + 'either decided by a platform admin or a tenant admin of its organization (the approve ' + + 'or reject is recorded with `via_override: true` and resumes the run) or reassigned to ' + + 'the position\'s holder, who then decides it. Verify on a running app, as an admin: the ' + + 'pending list filtered by `approverId` set to each such slot address answers no rows.', +}; diff --git a/packages/spec/src/migrations/registry.ts b/packages/spec/src/migrations/registry.ts index ebd7062ca4e..8c1e4c792cc 100644 --- a/packages/spec/src/migrations/registry.ts +++ b/packages/spec/src/migrations/registry.ts @@ -7728,6 +7728,65 @@ const step18: MigrationStep = { + 'TTL is still refused. A route declared with `timeoutMs: 30000` aborts after 30 seconds ' + 'exactly as `timeout: 30000` did.', }, + { + id: 'approval-position-address-role-retired', + surface: + 'approvals position address role: — the approverId filter of the approvals ' + + 'request list, the actorId of every decision, and a stored pending_approvers slot', + replacement: + '`position:`, the one spelling of a position address; a flow approver authored ' + + 'as `{ type: \'role\', value: }` becomes ' + + '`{ type: \'position\', value: }`', + reason: + 'The fourth face of the ADR-0090 D3 `role` retirement, beside ' + + '`actor-user-roles-to-positions` and `action-session-roles-to-positions`, and like them ' + + 'a runtime face with no spec schema. The approvals service read `role:` as a ' + + 'second spelling of `position:` wherever it compares a slot with the caller ' + + '(the "My Pending" filter, the participant gate, `viewer.can_act`, and the slot test of ' + + 'every decision), because 15.x-era slots and the stock console\'s identity list carried ' + + 'it. ADR-0090 D3 retires the word with no alias window, so once the pinned console sent ' + + '`position:` the arm came out in one edit (maintainer ruling, 2026-10-04). ' + + '`position:` is now the only position address: a `role:` ask matches ' + + 'only a slot stored under that exact spelling, and a `role:` actor is refused ' + + 'with 403 `FORBIDDEN`. ' + + 'The same ruling closed the one WRITER of the spelling. The deprecated `role` approver ' + + 'TYPE already resolved as `org_membership_level` (the org-membership tier: owner, admin, ' + + 'member), but when that lookup found no one the fallback slot kept the AUTHORED spelling, ' + + '`role:`, and a holder of a same-named position decided it through the arm, so ' + + 'the runtime silently honoured a membership-tier declaration as a position. The fallback ' + + 'now writes the canonical `org_membership_level:`, and no path writes a `role:` ' + + 'slot. ' + + 'Two classes of pending request are therefore decided only by the privileged override, ' + + 'or by a reassign to a real approver: a request a 15.x-era release stored as ' + + '`role:`, and a new request from a flow that still authors ' + + '`{ type: \'role\', value: }` and whose tier lookup finds no one. No ' + + 'stored slot is rewritten: the ruling refused a one-time rewrite as the permanent ' + + 'migration debt ADR-0090\'s first forcing fact names. ' + + 'Why this is a D3 semantic TODO and not a D2 conversion, on two independent grounds. ' + + 'FIRST, no metadata key moves: the address is runtime DATA (a request slot, a query ' + + 'parameter, a decision body\'s actor), never a `sys_metadata` row, so there is no source ' + + 'for a declarative transform to rewrite. SECOND, the one authored shape that leads here, ' + + '`{ type: \'role\', value: }`, is ambiguous by construction: the deprecated alias ' + + 'means the membership TIER, and whether its author meant a position instead is a ' + + 'judgment only that author can make, so a mechanical rewrite to either type would guess. ' + + 'The `ApproverType` `role` alias itself is a separate retirement and is unchanged here. ' + + 'ADR-0090 D3, ADR-0087.', + acceptanceCriteria: + 'No client sends `role:` as an `approverId` or an `actorId`: every such value is ' + + '`position:`, and a request routed to the position is listed for its holder, ' + + 'served with `viewer.can_act: true`, and decided by that holder with no `actorId` named. ' + + 'Every flow approver authored as `{ type: \'role\', value: }` is reviewed by its ' + + 'author: `` a position name becomes `{ type: \'position\', value: }`, and `` a ' + + 'membership tier (owner, admin, member) becomes ' + + '`{ type: \'org_membership_level\', value: }`. `os lint` reports the first as ' + + '`approval-approver-not-membership-tier` and the second as ' + + '`approval-approver-type-deprecated`. Every pending request whose slot reads ' + + '`role:`, or `org_membership_level:` from such a flow, is ' + + 'either decided by a platform admin or a tenant admin of its organization (the approve ' + + 'or reject is recorded with `via_override: true` and resumes the run) or reassigned to ' + + 'the position\'s holder, who then decides it. Verify on a running app, as an admin: the ' + + 'pending list filtered by `approverId` set to each such slot address answers no rows.', + }, // #15219 — maintainer ruling A for both keys (2026-09-04, director relay, // verbatim 「同意」): `plugins` and `devPlugins` are artifact ENVELOPE keys — // top level only, never inside `packages[]`. Registered as D3 SEMANTIC and