From 33df5e7107251366b67fdfaaa1375c6a4f500bd5 Mon Sep 17 00:00:00 2001 From: Claude Date: Sun, 4 Oct 2026 16:37:04 +0000 Subject: [PATCH 1/6] fix(plugin-approvals): retire the role: position-address arm and canonicalize the fallback literal (WIP) position: is the one spelling of a position address (ADR-0090 D3); the deprecated role approver type's empty-lookup fallback writes the canonical org_membership_level: literal. Pins turned; enumeration and admin-rescue pins added. Claude-Session: https://claude.ai/code/session_011K3zqE8Pv1Evw5hc8tZCnN Co-authored-by: Claude --- .../src/approval-service.test.ts | 288 +++++++++++++++--- .../plugin-approvals/src/approval-service.ts | 58 ++-- .../src/approver-address-readers.test.ts | 166 ++++++++++ .../src/approver-address.test.ts | 40 +-- .../plugin-approvals/src/approver-address.ts | 53 ++-- .../src/sys-approval-approver.object.ts | 4 +- 6 files changed, 494 insertions(+), 115 deletions(-) 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..4dc8c237681 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,83 @@ 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}:` — the `:` shape. */ +function typeValueTemplates(sf: ts.SourceFile): ts.TemplateExpression[] { + const out: ts.TemplateExpression[] = []; + const visit = (node: ts.Node): void => { + if (ts.isTemplateExpression(node) && node.head.text === '' && node.templateSpans[0]?.literal.text.startsWith(':')) { + 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 +434,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}`]; }', + ].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', }), From 6da63750870c5b0c0135ba4300dad04a058ac42b Mon Sep 17 00:00:00 2001 From: Claude Date: Sun, 4 Oct 2026 16:39:19 +0000 Subject: [PATCH 2/6] wip: changeset draft Claude-Session: https://claude.ai/code/session_011K3zqE8Pv1Evw5hc8tZCnN Co-authored-by: Claude --- .changeset/21387-retire-role-arm.md | 24 ++++++++++++++++++++++++ 1 file changed, 24 insertions(+) create mode 100644 .changeset/21387-retire-role-arm.md diff --git a/.changeset/21387-retire-role-arm.md b/.changeset/21387-retire-role-arm.md new file mode 100644 index 00000000000..e8aa2de3fb8 --- /dev/null +++ b/.changeset/21387-retire-role-arm.md @@ -0,0 +1,24 @@ +--- +"@objectstack/plugin-approvals": minor +--- + +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. + + From bce0f4a25bbaf83a89844b9a1dc088810b0692de Mon Sep 17 00:00:00 2001 From: Claude Date: Sun, 4 Oct 2026 16:41:21 +0000 Subject: [PATCH 3/6] test(dogfood), docs: role: rows flip to refusals; approvals guide names position: as the one address Claude-Session: https://claude.ai/code/session_011K3zqE8Pv1Evw5hc8tZCnN Co-authored-by: Claude --- .changeset/21387-retire-role-arm.md | 2 - content/docs/automation/approvals.mdx | 39 ++++++++---- .../src/approver-address-readers.test.ts | 9 ++- ...y-pending-position-address.dogfood.test.ts | 61 +++++++++++-------- .../position-address-readers.dogfood.test.ts | 23 ++++--- 5 files changed, 85 insertions(+), 49 deletions(-) diff --git a/.changeset/21387-retire-role-arm.md b/.changeset/21387-retire-role-arm.md index e8aa2de3fb8..a4f34c33fe5 100644 --- a/.changeset/21387-retire-role-arm.md +++ b/.changeset/21387-retire-role-arm.md @@ -20,5 +20,3 @@ Two classes of pending request are now decided only by an admin override: **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..9c9ec2d39d4 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 pre-rename `role:` 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,12 @@ 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; `role:` 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 +629,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 +660,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 `role` word: a request a 15.x-era release stored as +`role:`, and a request opened from a flow that still authors +`{ type: 'role', 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/approver-address-readers.test.ts b/packages/plugins/plugin-approvals/src/approver-address-readers.test.ts index 4dc8c237681..6a17ea2913e 100644 --- a/packages/plugins/plugin-approvals/src/approver-address-readers.test.ts +++ b/packages/plugins/plugin-approvals/src/approver-address-readers.test.ts @@ -247,11 +247,14 @@ function literalPieces(sf: ts.SourceFile): Array<{ text: string; node: ts.Node } return out; } -/** Every template in `sf` that opens with `${X}:` — the `:` shape. */ +/** + * 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 === '' && node.templateSpans[0]?.literal.text.startsWith(':')) { + if (ts.isTemplateExpression(node) && node.head.text === '' && /^:(?!\s)/.test(node.templateSpans[0]?.literal.text ?? '')) { out.push(node); } ts.forEachChild(node, visit); @@ -494,7 +497,7 @@ describe('approver-address — the readers of the equivalence, and no comparison '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}`]; }', + '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']); 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. From 022bb0fac85c43a8ee1f6249693c5b6ec0695b54 Mon Sep 17 00:00:00 2001 From: Claude Date: Sun, 4 Oct 2026 17:04:46 +0000 Subject: [PATCH 4/6] docs(approvals): name the retired prefix without the reserved word (check:role-word) Claude-Session: https://claude.ai/code/session_011K3zqE8Pv1Evw5hc8tZCnN Co-authored-by: Claude --- content/docs/automation/approvals.mdx | 17 +++++++++-------- 1 file changed, 9 insertions(+), 8 deletions(-) diff --git a/content/docs/automation/approvals.mdx b/content/docs/automation/approvals.mdx index 9c9ec2d39d4..0925c9662ca 100644 --- a/content/docs/automation/approvals.mdx +++ b/content/docs/automation/approvals.mdx @@ -422,7 +422,7 @@ curl -b cookies.txt \ 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 `position:`, the one spelling the service admits a holder of that -position under as their own identity. The pre-rename `role:` prefix is no +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`, @@ -510,8 +510,9 @@ curl -b cookies.txt -X POST \ 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; `role:` is not -one of them (ADR-0090 D3) and returns 403 (`FORBIDDEN: cannot act as '…'`). No +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` @@ -661,11 +662,11 @@ 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 `role` word: a request a 15.x-era release stored as -`role:`, and a request opened from a flow that still authors -`{ type: 'role', 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 +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 From 73977ceb7e87585b33b30e695b21a850f2beebe9 Mon Sep 17 00:00:00 2001 From: Claude Date: Sun, 4 Oct 2026 17:39:02 +0000 Subject: [PATCH 5/6] feat(spec): register the ADR-0090 D3 approvals position-address retirement in the ADR-0087 ledger One D3 semantic entry, approval-position-address-role-retired, with its generated registry region, spec-changes.json and upgrade-guide rows; the changeset's disposition marker names it. Claude-Session: https://claude.ai/code/session_011K3zqE8Pv1Evw5hc8tZCnN Co-authored-by: Claude --- .changeset/21387-retire-role-arm.md | 2 + docs/protocol-upgrade-guide.md | 3 + packages/spec/spec-changes.json | 14 +++++ ....approval-position-address-role-retired.ts | 63 +++++++++++++++++++ packages/spec/src/migrations/registry.ts | 59 +++++++++++++++++ 5 files changed, 141 insertions(+) create mode 100644 packages/spec/src/migrations/entries/semantic/17.approval-position-address-role-retired.ts diff --git a/.changeset/21387-retire-role-arm.md b/.changeset/21387-retire-role-arm.md index a4f34c33fe5..5ad1263b286 100644 --- a/.changeset/21387-retire-role-arm.md +++ b/.changeset/21387-retire-role-arm.md @@ -20,3 +20,5 @@ Two classes of pending request are now decided only by an admin override: **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/docs/protocol-upgrade-guide.md b/docs/protocol-upgrade-guide.md index 5c98d9147ba..9429556ae60 100644 --- a/docs/protocol-upgrade-guide.md +++ b/docs/protocol-upgrade-guide.md @@ -239,6 +239,9 @@ Finally it removes the 'pdf' member of `view.exportOptions` formats (maintainer - **`approval-escalation-enabled-default-flip`** — `automation.ApprovalEscalation.enabled — an OMITTED value inside an approval node's escalation block` → nothing, for the common intent (escalate on timeout): an escalation block carrying timeoutHours is live by default. To declare an SLA OFF while keeping its configuration, write enabled: false explicitly — which is now the spelling the escalation sweep actually reads - Why not automatic: A DECLARED-DEFAULT CORRECTION plus the enforcement that makes the key real (maintainer ruling 2026-08-27, which moved the declared default to what the sweep had always done) — the same category as protocol 17's `import-run-automations-declared-default-corrected`: the schema promised `enabled` defaults to `false` (SLA off) while the plugin-approvals sweep never read the key at all — any escalation block with a positive `timeoutHours` escalated, and with `action: 'auto_approve'` that silently approved requests their author had declared off the clock. The flip moves the default to `true` and, in the same change, the sweep starts honouring an explicit `enabled: false`. The feature-level switch is whether an `escalation` block exists at all; within a block carrying `timeoutHours`, escalation is on unless explicitly turned off. Deployed metadata that OMITS `enabled` does not change behaviour: it escalated before (the sweep ignored the key) and escalates after (the parse materializes `true`). Stored request snapshots written before the flip carry a MATERIALIZED `enabled: false` (the approval-node executor parses config through the old schema before snapshotting), so the sweep keeps a read-side legacy window keyed on the snapshot's `created_at`: pre-flip snapshots keep escalating exactly as they do today, and the window retires itself as those pending requests drain. What DOES change is that an explicit `enabled: false` finally binds — a flow that authored it (e.g. the console toggle switched off after a timeout was set) stops escalating on requests opened after the upgrade, which is the declared intent being honoured. - Done when: A flow whose approval node omits `enabled` inside `escalation` still escalates on timeout (no metadata edit needed). A flow that writes `enabled: false` stops escalating for newly opened requests — verify one such request stays pending past its `timeoutHours` with no `escalate` audit row and no auto-decision. Requests opened BEFORE the upgrade keep their pre-upgrade behaviour (they escalate) regardless of the stored `enabled` bit. Clients that parse metadata through the published JSON Schema now materialize `enabled: true` where they materialized `false`; a client that needs the SLA off must write it explicitly. +- **`approval-position-address-role-retired`** — `approvals position address role: — the approverId filter of the approvals request list, the actorId of every decision, and a stored pending_approvers slot` → `position:`, the one spelling of a position address; a flow approver authored as `{ type: 'role', value: }` becomes `{ type: 'position', value: }` + - Why not automatic: 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. + - Done when: 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. - **`audit-log-action-enum-retired`** — `sys_audit_log.action — the values 'export' and 'permission_change' left the select enum declared by plugin-audit (packages/plugins/plugin-audit/src/objects/sys-audit-log.object.ts). The same two values also left the shipped list-view filters on that object: 'permission_change' from the auth_events view and 'export' from the config_changes view` → nothing, for either value — both are removed rather than renamed, because neither named an event this platform records. For permission changes, read the ordinary `create` / `update` rows on the permission objects themselves: a grant or binding write is an ordinary record write and the generic audit writer already ledgers it, so a second semantically-duplicate row was never minted. For `export` there is no replacement and nothing is lost: no export feature ever wrote an audit row. A consumer filtering `sys_audit_log` on either value was reading an empty result set on every deployment, and still is — what changed is that the contract no longer promises otherwise - Why not automatic: Maintainer ruling 2026-08-12 on the audit log's writerless actions, the retirement half of a two-half verdict: the cheap writers get built (`login` / `logout` on the auth session hooks, `config_change` from the settings service) and the enum values with no feature behind them are retired. 原则记录:空 widget + 永远查不到东西的过滤器是可见产品缺陷;审计面宁窄勿谎. The defect was false compliance on a COMPLIANCE surface, which is the sharpest form of ADR-0049 declared-≠-enforced: an auditor reading the action enum believed the platform captured permission changes and data exports, and the shipped list views and dashboard widgets showed them a filter and a tile for exactly those events. Both were permanently empty. Measured by enumerating every `sys_audit_log` writer in the repo — there are exactly two: plugin-audit`s generic hook writer, whose `actionFor` maps afterInsert/Update/Delete to create/update/delete and nothing else, and plugin-auth`s admin user-import. Neither has ever emitted `export` or `permission_change`. This is an enum-VALUE retirement, so the bookkeeping differs from a key retirement in the two ways `hook-body-crypto-hash-removed`, `dataset-measure-array-string-agg-removed` and `action-global-nav-location-removed` already record: nothing lands in RETIRED_KEYS_BY_MAJOR (no authorable KEY changed) and the four surface ratchets are expected to be byte-identical (no def changed). It differs from all three in being a SEMANTIC entry rather than a D2 conversion, and the reason is that there is no source to rewrite: `sys_audit_log` is a platform-owned, append-only object whose every field is `readonly: true`. Nobody authors an audit row and nobody authors this enum — the values appear only in rows the runtime writes and in queries consumers send. A conversion rewrites authored metadata or a stored `sys_metadata` row; this surface is neither, so the disposition is the one `BatchOptions.validateOnly` and the notification cursor already take in this major. ⚠️ Historical ROWS are deliberately untouched. A deployment that somehow holds a row with either value keeps it, and keeps reading it back: the enum is not enforced on this object at all (`validateRecord` skips `readonly` fields, and every field here is readonly), so nothing rejects stored history and no backfill is required or wanted. Deleting audit history to satisfy a schema narrowing would be the one genuinely destructive reading of this change. ADR-0049 / ADR-0087. - Done when: No consumer filters `sys_audit_log` on `action = "export"` or `action = "permission_change"` expecting rows: both were empty everywhere before this change, so a query that returned data has not been identified and a query that returned nothing behaves identically. Concretely, check three places. (1) Saved queries, dashboards and reports over `sys_audit_log`: a filter naming either value should be deleted, not re-pointed — for permission auditing, filter the permission objects` own `create`/`update` rows by `object_name` instead. (2) Any code branching on the action string (a badge map, a label switch, an `if (row.action === ...)`): the arms for these two values are now unreachable and should go, and a `switch` with an exhaustiveness check over the enum type will now fail to compile if they stay — that compile error is the enforced channel for TypeScript consumers. (3) Custom objects or plugins inserting `sys_audit_log` rows with either value: this is the only case that needs a real decision, because the write will NOT be refused (readonly fields are not validated) — it will simply be a row whose action the object no longer declares. Pick a declared value or open an issue for the action you actually need. ⚠️ Do NOT migrate or delete existing rows: audit history is append-only and stays exactly as written. diff --git a/packages/spec/spec-changes.json b/packages/spec/spec-changes.json index 14b08dbbd9c..843c52897a9 100644 --- a/packages/spec/spec-changes.json +++ b/packages/spec/spec-changes.json @@ -422,6 +422,13 @@ "toMajor": 17, "rationale": "A DECLARED-DEFAULT CORRECTION plus the enforcement that makes the key real (maintainer ruling 2026-08-27, which moved the declared default to what the sweep had always done) — the same category as protocol 17's `import-run-automations-declared-default-corrected`: the schema promised `enabled` defaults to `false` (SLA off) while the plugin-approvals sweep never read the key at all — any escalation block with a positive `timeoutHours` escalated, and with `action: 'auto_approve'` that silently approved requests their author had declared off the clock. The flip moves the default to `true` and, in the same change, the sweep starts honouring an explicit `enabled: false`. The feature-level switch is whether an `escalation` block exists at all; within a block carrying `timeoutHours`, escalation is on unless explicitly turned off. Deployed metadata that OMITS `enabled` does not change behaviour: it escalated before (the sweep ignored the key) and escalates after (the parse materializes `true`). Stored request snapshots written before the flip carry a MATERIALIZED `enabled: false` (the approval-node executor parses config through the old schema before snapshotting), so the sweep keeps a read-side legacy window keyed on the snapshot's `created_at`: pre-flip snapshots keep escalating exactly as they do today, and the window retires itself as those pending requests drain. What DOES change is that an explicit `enabled: false` finally binds — a flow that authored it (e.g. the console toggle switched off after a timeout was set) stops escalating on requests opened after the upgrade, which is the declared intent being honoured." }, + { + "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: }`", + "migrationId": "approval-position-address-role-retired", + "toMajor": 17, + "rationale": "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." + }, { "surface": "sys_audit_log.action — the values 'export' and 'permission_change' left the select enum declared by plugin-audit (packages/plugins/plugin-audit/src/objects/sys-audit-log.object.ts). The same two values also left the shipped list-view filters on that object: 'permission_change' from the auth_events view and 'export' from the config_changes view", "replacement": "nothing, for either value — both are removed rather than renamed, because neither named an event this platform records. For permission changes, read the ordinary `create` / `update` rows on the permission objects themselves: a grant or binding write is an ordinary record write and the generic audit writer already ledgers it, so a second semantically-duplicate row was never minted. For `export` there is no replacement and nothing is lost: no export feature ever wrote an audit row. A consumer filtering `sys_audit_log` on either value was reading an empty result set on every deployment, and still is — what changed is that the contract no longer promises otherwise", @@ -1314,6 +1321,13 @@ "toMajor": 17, "rationale": "A DECLARED-DEFAULT CORRECTION plus the enforcement that makes the key real (maintainer ruling 2026-08-27, which moved the declared default to what the sweep had always done) — the same category as protocol 17's `import-run-automations-declared-default-corrected`: the schema promised `enabled` defaults to `false` (SLA off) while the plugin-approvals sweep never read the key at all — any escalation block with a positive `timeoutHours` escalated, and with `action: 'auto_approve'` that silently approved requests their author had declared off the clock. The flip moves the default to `true` and, in the same change, the sweep starts honouring an explicit `enabled: false`. The feature-level switch is whether an `escalation` block exists at all; within a block carrying `timeoutHours`, escalation is on unless explicitly turned off. Deployed metadata that OMITS `enabled` does not change behaviour: it escalated before (the sweep ignored the key) and escalates after (the parse materializes `true`). Stored request snapshots written before the flip carry a MATERIALIZED `enabled: false` (the approval-node executor parses config through the old schema before snapshotting), so the sweep keeps a read-side legacy window keyed on the snapshot's `created_at`: pre-flip snapshots keep escalating exactly as they do today, and the window retires itself as those pending requests drain. What DOES change is that an explicit `enabled: false` finally binds — a flow that authored it (e.g. the console toggle switched off after a timeout was set) stops escalating on requests opened after the upgrade, which is the declared intent being honoured." }, + { + "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: }`", + "migrationId": "approval-position-address-role-retired", + "toMajor": 17, + "rationale": "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." + }, { "surface": "sys_audit_log.action — the values 'export' and 'permission_change' left the select enum declared by plugin-audit (packages/plugins/plugin-audit/src/objects/sys-audit-log.object.ts). The same two values also left the shipped list-view filters on that object: 'permission_change' from the auth_events view and 'export' from the config_changes view", "replacement": "nothing, for either value — both are removed rather than renamed, because neither named an event this platform records. For permission changes, read the ordinary `create` / `update` rows on the permission objects themselves: a grant or binding write is an ordinary record write and the generic audit writer already ledgers it, so a second semantically-duplicate row was never minted. For `export` there is no replacement and nothing is lost: no export feature ever wrote an audit row. A consumer filtering `sys_audit_log` on either value was reading an empty result set on every deployment, and still is — what changed is that the contract no longer promises otherwise", diff --git a/packages/spec/src/migrations/entries/semantic/17.approval-position-address-role-retired.ts b/packages/spec/src/migrations/entries/semantic/17.approval-position-address-role-retired.ts new file mode 100644 index 00000000000..e3840cb7afd --- /dev/null +++ b/packages/spec/src/migrations/entries/semantic/17.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 6413a91ee4e..02bfff699b0 100644 --- a/packages/spec/src/migrations/registry.ts +++ b/packages/spec/src/migrations/registry.ts @@ -1620,6 +1620,65 @@ const step17: MigrationStep = { + '`enabled: true` where they materialized `false`; a client that needs the SLA ' + 'off must write it explicitly.', }, + { + 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.', + }, { id: 'audit-log-action-enum-retired', // No backticks in `surface` — build-upgrade-guide.ts renders it inside a From 811c17009a7d1aa3efc60ec5be529f54546672f6 Mon Sep 17 00:00:00 2001 From: Claude Date: Sun, 4 Oct 2026 19:28:06 +0000 Subject: [PATCH 6/6] fix(spec): file the approvals position-address retirement under step 18, and name spec in the changeset The ledger entry approval-position-address-role-retired moves from step 17 to the uncut step 18 (the step18 docblock: a narrowing after the v17.0.0 cut is told where os migrate meta --from 17 looks); id unchanged. The registry is regenerated, and spec-changes.json and the upgrade guide return to main's bytes (both project only up to PROTOCOL_MAJOR). The changeset names @objectstack/spec at patch. One fixture comment stops claiming both spellings. Claude-Session: https://claude.ai/code/session_011K3zqE8Pv1Evw5hc8tZCnN Co-authored-by: Claude --- .changeset/21387-retire-role-arm.md | 1 + docs/protocol-upgrade-guide.md | 3 - .../fixtures/my-pending-position-fixture.ts | 3 +- packages/spec/spec-changes.json | 14 --- ...approval-position-address-role-retired.ts} | 0 packages/spec/src/migrations/registry.ts | 118 +++++++++--------- 6 files changed, 62 insertions(+), 77 deletions(-) rename packages/spec/src/migrations/entries/semantic/{17.approval-position-address-role-retired.ts => 18.approval-position-address-role-retired.ts} (100%) diff --git a/.changeset/21387-retire-role-arm.md b/.changeset/21387-retire-role-arm.md index 5ad1263b286..7e3e89263b4 100644 --- a/.changeset/21387-retire-role-arm.md +++ b/.changeset/21387-retire-role-arm.md @@ -1,5 +1,6 @@ --- "@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) diff --git a/docs/protocol-upgrade-guide.md b/docs/protocol-upgrade-guide.md index 9429556ae60..5c98d9147ba 100644 --- a/docs/protocol-upgrade-guide.md +++ b/docs/protocol-upgrade-guide.md @@ -239,9 +239,6 @@ Finally it removes the 'pdf' member of `view.exportOptions` formats (maintainer - **`approval-escalation-enabled-default-flip`** — `automation.ApprovalEscalation.enabled — an OMITTED value inside an approval node's escalation block` → nothing, for the common intent (escalate on timeout): an escalation block carrying timeoutHours is live by default. To declare an SLA OFF while keeping its configuration, write enabled: false explicitly — which is now the spelling the escalation sweep actually reads - Why not automatic: A DECLARED-DEFAULT CORRECTION plus the enforcement that makes the key real (maintainer ruling 2026-08-27, which moved the declared default to what the sweep had always done) — the same category as protocol 17's `import-run-automations-declared-default-corrected`: the schema promised `enabled` defaults to `false` (SLA off) while the plugin-approvals sweep never read the key at all — any escalation block with a positive `timeoutHours` escalated, and with `action: 'auto_approve'` that silently approved requests their author had declared off the clock. The flip moves the default to `true` and, in the same change, the sweep starts honouring an explicit `enabled: false`. The feature-level switch is whether an `escalation` block exists at all; within a block carrying `timeoutHours`, escalation is on unless explicitly turned off. Deployed metadata that OMITS `enabled` does not change behaviour: it escalated before (the sweep ignored the key) and escalates after (the parse materializes `true`). Stored request snapshots written before the flip carry a MATERIALIZED `enabled: false` (the approval-node executor parses config through the old schema before snapshotting), so the sweep keeps a read-side legacy window keyed on the snapshot's `created_at`: pre-flip snapshots keep escalating exactly as they do today, and the window retires itself as those pending requests drain. What DOES change is that an explicit `enabled: false` finally binds — a flow that authored it (e.g. the console toggle switched off after a timeout was set) stops escalating on requests opened after the upgrade, which is the declared intent being honoured. - Done when: A flow whose approval node omits `enabled` inside `escalation` still escalates on timeout (no metadata edit needed). A flow that writes `enabled: false` stops escalating for newly opened requests — verify one such request stays pending past its `timeoutHours` with no `escalate` audit row and no auto-decision. Requests opened BEFORE the upgrade keep their pre-upgrade behaviour (they escalate) regardless of the stored `enabled` bit. Clients that parse metadata through the published JSON Schema now materialize `enabled: true` where they materialized `false`; a client that needs the SLA off must write it explicitly. -- **`approval-position-address-role-retired`** — `approvals position address role: — the approverId filter of the approvals request list, the actorId of every decision, and a stored pending_approvers slot` → `position:`, the one spelling of a position address; a flow approver authored as `{ type: 'role', value: }` becomes `{ type: 'position', value: }` - - Why not automatic: 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. - - Done when: 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. - **`audit-log-action-enum-retired`** — `sys_audit_log.action — the values 'export' and 'permission_change' left the select enum declared by plugin-audit (packages/plugins/plugin-audit/src/objects/sys-audit-log.object.ts). The same two values also left the shipped list-view filters on that object: 'permission_change' from the auth_events view and 'export' from the config_changes view` → nothing, for either value — both are removed rather than renamed, because neither named an event this platform records. For permission changes, read the ordinary `create` / `update` rows on the permission objects themselves: a grant or binding write is an ordinary record write and the generic audit writer already ledgers it, so a second semantically-duplicate row was never minted. For `export` there is no replacement and nothing is lost: no export feature ever wrote an audit row. A consumer filtering `sys_audit_log` on either value was reading an empty result set on every deployment, and still is — what changed is that the contract no longer promises otherwise - Why not automatic: Maintainer ruling 2026-08-12 on the audit log's writerless actions, the retirement half of a two-half verdict: the cheap writers get built (`login` / `logout` on the auth session hooks, `config_change` from the settings service) and the enum values with no feature behind them are retired. 原则记录:空 widget + 永远查不到东西的过滤器是可见产品缺陷;审计面宁窄勿谎. The defect was false compliance on a COMPLIANCE surface, which is the sharpest form of ADR-0049 declared-≠-enforced: an auditor reading the action enum believed the platform captured permission changes and data exports, and the shipped list views and dashboard widgets showed them a filter and a tile for exactly those events. Both were permanently empty. Measured by enumerating every `sys_audit_log` writer in the repo — there are exactly two: plugin-audit`s generic hook writer, whose `actionFor` maps afterInsert/Update/Delete to create/update/delete and nothing else, and plugin-auth`s admin user-import. Neither has ever emitted `export` or `permission_change`. This is an enum-VALUE retirement, so the bookkeeping differs from a key retirement in the two ways `hook-body-crypto-hash-removed`, `dataset-measure-array-string-agg-removed` and `action-global-nav-location-removed` already record: nothing lands in RETIRED_KEYS_BY_MAJOR (no authorable KEY changed) and the four surface ratchets are expected to be byte-identical (no def changed). It differs from all three in being a SEMANTIC entry rather than a D2 conversion, and the reason is that there is no source to rewrite: `sys_audit_log` is a platform-owned, append-only object whose every field is `readonly: true`. Nobody authors an audit row and nobody authors this enum — the values appear only in rows the runtime writes and in queries consumers send. A conversion rewrites authored metadata or a stored `sys_metadata` row; this surface is neither, so the disposition is the one `BatchOptions.validateOnly` and the notification cursor already take in this major. ⚠️ Historical ROWS are deliberately untouched. A deployment that somehow holds a row with either value keeps it, and keeps reading it back: the enum is not enforced on this object at all (`validateRecord` skips `readonly` fields, and every field here is readonly), so nothing rejects stored history and no backfill is required or wanted. Deleting audit history to satisfy a schema narrowing would be the one genuinely destructive reading of this change. ADR-0049 / ADR-0087. - Done when: No consumer filters `sys_audit_log` on `action = "export"` or `action = "permission_change"` expecting rows: both were empty everywhere before this change, so a query that returned data has not been identified and a query that returned nothing behaves identically. Concretely, check three places. (1) Saved queries, dashboards and reports over `sys_audit_log`: a filter naming either value should be deleted, not re-pointed — for permission auditing, filter the permission objects` own `create`/`update` rows by `object_name` instead. (2) Any code branching on the action string (a badge map, a label switch, an `if (row.action === ...)`): the arms for these two values are now unreachable and should go, and a `switch` with an exhaustiveness check over the enum type will now fail to compile if they stay — that compile error is the enforced channel for TypeScript consumers. (3) Custom objects or plugins inserting `sys_audit_log` rows with either value: this is the only case that needs a real decision, because the write will NOT be refused (readonly fields are not validated) — it will simply be a row whose action the object no longer declares. Pick a declared value or open an issue for the action you actually need. ⚠️ Do NOT migrate or delete existing rows: audit history is append-only and stays exactly as written. 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/spec/spec-changes.json b/packages/spec/spec-changes.json index 843c52897a9..14b08dbbd9c 100644 --- a/packages/spec/spec-changes.json +++ b/packages/spec/spec-changes.json @@ -422,13 +422,6 @@ "toMajor": 17, "rationale": "A DECLARED-DEFAULT CORRECTION plus the enforcement that makes the key real (maintainer ruling 2026-08-27, which moved the declared default to what the sweep had always done) — the same category as protocol 17's `import-run-automations-declared-default-corrected`: the schema promised `enabled` defaults to `false` (SLA off) while the plugin-approvals sweep never read the key at all — any escalation block with a positive `timeoutHours` escalated, and with `action: 'auto_approve'` that silently approved requests their author had declared off the clock. The flip moves the default to `true` and, in the same change, the sweep starts honouring an explicit `enabled: false`. The feature-level switch is whether an `escalation` block exists at all; within a block carrying `timeoutHours`, escalation is on unless explicitly turned off. Deployed metadata that OMITS `enabled` does not change behaviour: it escalated before (the sweep ignored the key) and escalates after (the parse materializes `true`). Stored request snapshots written before the flip carry a MATERIALIZED `enabled: false` (the approval-node executor parses config through the old schema before snapshotting), so the sweep keeps a read-side legacy window keyed on the snapshot's `created_at`: pre-flip snapshots keep escalating exactly as they do today, and the window retires itself as those pending requests drain. What DOES change is that an explicit `enabled: false` finally binds — a flow that authored it (e.g. the console toggle switched off after a timeout was set) stops escalating on requests opened after the upgrade, which is the declared intent being honoured." }, - { - "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: }`", - "migrationId": "approval-position-address-role-retired", - "toMajor": 17, - "rationale": "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." - }, { "surface": "sys_audit_log.action — the values 'export' and 'permission_change' left the select enum declared by plugin-audit (packages/plugins/plugin-audit/src/objects/sys-audit-log.object.ts). The same two values also left the shipped list-view filters on that object: 'permission_change' from the auth_events view and 'export' from the config_changes view", "replacement": "nothing, for either value — both are removed rather than renamed, because neither named an event this platform records. For permission changes, read the ordinary `create` / `update` rows on the permission objects themselves: a grant or binding write is an ordinary record write and the generic audit writer already ledgers it, so a second semantically-duplicate row was never minted. For `export` there is no replacement and nothing is lost: no export feature ever wrote an audit row. A consumer filtering `sys_audit_log` on either value was reading an empty result set on every deployment, and still is — what changed is that the contract no longer promises otherwise", @@ -1321,13 +1314,6 @@ "toMajor": 17, "rationale": "A DECLARED-DEFAULT CORRECTION plus the enforcement that makes the key real (maintainer ruling 2026-08-27, which moved the declared default to what the sweep had always done) — the same category as protocol 17's `import-run-automations-declared-default-corrected`: the schema promised `enabled` defaults to `false` (SLA off) while the plugin-approvals sweep never read the key at all — any escalation block with a positive `timeoutHours` escalated, and with `action: 'auto_approve'` that silently approved requests their author had declared off the clock. The flip moves the default to `true` and, in the same change, the sweep starts honouring an explicit `enabled: false`. The feature-level switch is whether an `escalation` block exists at all; within a block carrying `timeoutHours`, escalation is on unless explicitly turned off. Deployed metadata that OMITS `enabled` does not change behaviour: it escalated before (the sweep ignored the key) and escalates after (the parse materializes `true`). Stored request snapshots written before the flip carry a MATERIALIZED `enabled: false` (the approval-node executor parses config through the old schema before snapshotting), so the sweep keeps a read-side legacy window keyed on the snapshot's `created_at`: pre-flip snapshots keep escalating exactly as they do today, and the window retires itself as those pending requests drain. What DOES change is that an explicit `enabled: false` finally binds — a flow that authored it (e.g. the console toggle switched off after a timeout was set) stops escalating on requests opened after the upgrade, which is the declared intent being honoured." }, - { - "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: }`", - "migrationId": "approval-position-address-role-retired", - "toMajor": 17, - "rationale": "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." - }, { "surface": "sys_audit_log.action — the values 'export' and 'permission_change' left the select enum declared by plugin-audit (packages/plugins/plugin-audit/src/objects/sys-audit-log.object.ts). The same two values also left the shipped list-view filters on that object: 'permission_change' from the auth_events view and 'export' from the config_changes view", "replacement": "nothing, for either value — both are removed rather than renamed, because neither named an event this platform records. For permission changes, read the ordinary `create` / `update` rows on the permission objects themselves: a grant or binding write is an ordinary record write and the generic audit writer already ledgers it, so a second semantically-duplicate row was never minted. For `export` there is no replacement and nothing is lost: no export feature ever wrote an audit row. A consumer filtering `sys_audit_log` on either value was reading an empty result set on every deployment, and still is — what changed is that the contract no longer promises otherwise", diff --git a/packages/spec/src/migrations/entries/semantic/17.approval-position-address-role-retired.ts b/packages/spec/src/migrations/entries/semantic/18.approval-position-address-role-retired.ts similarity index 100% rename from packages/spec/src/migrations/entries/semantic/17.approval-position-address-role-retired.ts rename to packages/spec/src/migrations/entries/semantic/18.approval-position-address-role-retired.ts diff --git a/packages/spec/src/migrations/registry.ts b/packages/spec/src/migrations/registry.ts index 4cc89285c9e..8c1e4c792cc 100644 --- a/packages/spec/src/migrations/registry.ts +++ b/packages/spec/src/migrations/registry.ts @@ -1620,65 +1620,6 @@ const step17: MigrationStep = { + '`enabled: true` where they materialized `false`; a client that needs the SLA ' + 'off must write it explicitly.', }, - { - 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.', - }, { id: 'audit-log-action-enum-retired', // No backticks in `surface` — build-upgrade-guide.ts renders it inside a @@ -7787,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