fix(approvals): admin override to recover an approval routed to an unstaffed position (#3424)

.changeset/approval-empty-position-admin-override.md

@@ -0,0 +1,34 @@
+---
+"@objectstack/plugin-approvals": patch
+"@objectstack/spec": patch
+---
+
+fix(approvals): admin override for a request routed to an unstaffed approver (#3424)
+
+An `approval` node routed to a `position` (or `team`/`department`) with **no
+holders** resolved to only the unresolvable `position:<name>` literal in
+`pending_approvers` — no concrete user was in the slate. Every normal
+`decide` / `reassign` / `recall` then returned `FORBIDDEN` (not a pending
+approver) and, with `lockRecord`, the target record stayed `RECORD_LOCKED`
+forever: a data-availability dead-end with no in-product recovery (the only exit
+was editing the DB by hand). Very easy to hit in fresh/demo orgs (positions
+seeded, holders not) and whenever a role is vacated in production.
+
+A **platform or tenant admin** — the same posture the engine's superuser bypass
+already trusts — may now act on any *pending* request to release it: **approve,
+reject, reassign** it to a real approver, or **recall** it. The override finalizes
+the request (which releases the record lock, keyed on a pending request); a
+tenant admin's authority is org-scoped, a platform admin's is not, and the
+decision is audited under the admin's own id. An admin approval is authoritative,
+finalizing the node even under `unanimous` / `quorum` / `per_group` rather than
+counting as one vote among the (empty) slate.
+
+- `sys_approval_request.viewer` gains `can_override` (server-computed): true for a
+  privileged admin on a pending request. The `approve` / `reject` / `reassign`
+  declared actions OR it into their `visible` gate, so the console surfaces the
+  recovery path without a hand-wired button. Existing approver/submitter gating is
+  unchanged.
+- `openNodeRequest` now logs a loud warning when a node resolves to **no concrete
+  approver**, so the misconfiguration is visible instead of silently locking the
+  record. The literal-fallback behavior (kept for 15.x slot back-compat) is
+  otherwise unchanged.

content/docs/automation/approvals.mdx

@@ -281,20 +281,38 @@ service attaches to every request it returns:
 
 ```jsonc
 // getRequest / listRequests responses carry, per the calling user:
-"viewer": { "can_act": true, "is_submitter": false }
+"viewer": { "can_act": true, "is_submitter": false, "can_override": false }
 ```
 
 - `can_act` — the caller is a **current pending approver** (their id is in the
   resolved `pending_approvers` while the request is `pending`). This is the same
   check the decision routes authorize with, so it already reflects
   position/team/manager resolution.
 - `is_submitter` — the caller submitted the request.
+- `can_override` — the caller is a **platform or tenant admin** who may act on a
+  `pending` request despite holding no slot (see the admin-override callout
+  below).
 
 Approver actions gate on `record.viewer.can_act`, submitter levers
-(remind/recall/resubmit) on `record.viewer.is_submitter`. So a submitter viewing
+(remind/recall/resubmit) on `record.viewer.is_submitter`, and Approve/Reject/
+Reassign additionally OR in `record.viewer.can_override`. So a submitter viewing
 their own pending request never sees Approve/Reject/Reassign (buttons the server
-would 403 anyway), and a position-addressed approver is never wrongly hidden.
-The service stays the sole authority — the predicate only trims the UI.
+would 403 anyway), a position-addressed approver is never wrongly hidden, and an
+admin can rescue a stuck request. The service stays the sole authority — the
+predicate only trims the UI.
+
+<Callout type="info">
+**Admin override — recovering a stuck request.** An approval routed to a
+`position` / `team` / `department` with **no holders** resolves to only an
+unresolvable `position:<name>` literal in `pending_approvers`: no concrete user
+can act, and (with `lockRecord`) the record stays locked. A **platform admin**
+(`admin_full_access`) or **tenant admin** (`organization_admin`, org-scoped) may
+act on any `pending` request — **approve, reject, reassign** it to a real
+approver, or **recall** it — releasing the lock. An admin decision is
+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.
+</Callout>
 
 ### Progress and notification deep links
 

examples/app-showcase/src/security/seed-approval-demo.ts

@@ -41,8 +41,16 @@ const SYS = { isSystem: true } as const;
 
 const ADMIN_EMAIL = 'admin@objectos.ai';
 
-/** Positions the admin is granted so they resolve as an approver on the demos. */
-const ADMIN_APPROVAL_POSITIONS = ['manager', 'finance', 'legal'] as const;
+/**
+ * Positions the admin is granted so they resolve as an approver on the demos.
+ * Covers every `{ type: 'position' }` the showcase approval flows route to and
+ * expect the admin to hold — including `exec`, the SECOND tier of
+ * `showcase_budget_approval` (manager → exec). Without `exec` a budget approval
+ * that a user drives to step 2 routes to an unstaffed position and dead-ends
+ * (framework#3424): undecidable request, record locked. Keep this in sync with
+ * the position values authored in `automation/flows`.
+ */
+const ADMIN_APPROVAL_POSITIONS = ['manager', 'finance', 'legal', 'exec'] as const;
 
 /** A phone-based demo persona (§6 "phone sign-in surfaces"). */
 const PHONE_DEMO_USER = {

packages/lint/src/index.ts

@@ -106,6 +106,7 @@ export {
   APPROVAL_APPROVER_TYPE_DEPRECATED,
   APPROVAL_APPROVER_TYPE_UNKNOWN,
   APPROVAL_ESCALATION_REASSIGN_NO_TARGET,
+  APPROVAL_APPROVERS_MAY_RESOLVE_EMPTY,
 } from './validate-approval-approvers.js';
 export type { ApprovalApproverFinding, ApprovalApproverSeverity } from './validate-approval-approvers.js';
 

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

@@ -7,6 +7,7 @@ import {
   APPROVAL_APPROVER_TYPE_DEPRECATED,
   APPROVAL_APPROVER_TYPE_UNKNOWN,
   APPROVAL_ESCALATION_REASSIGN_NO_TARGET,
+  APPROVAL_APPROVERS_MAY_RESOLVE_EMPTY,
 } from './validate-approval-approvers.js';
 
 function stackWithApprovers(approvers: unknown[]): Record<string, unknown> {
@@ -120,6 +121,56 @@ describe('validateApprovalApprovers', () => {
     expect(validateApprovalApprovers(stack)).toEqual([]);
   });
 
+  // ── empty-slate dead-end (#3424) ─────────────────────────────────────────
+
+  it('flags a node routed only to a single group (unstaffed-position dead-end)', () => {
+    const findings = validateApprovalApprovers(stackWithApprovers([
+      { type: 'position', value: 'exec' },
+    ]));
+    expect(findings).toHaveLength(1);
+    expect(findings[0].rule).toBe(APPROVAL_APPROVERS_MAY_RESOLVE_EMPTY);
+    expect(findings[0].severity).toBe('info');
+    expect(findings[0].path).toBe('flows[0].nodes[1].config.approvers');
+    expect(findings[0].message).toContain('locked'); // lockRecord defaults true
+    expect(findings[0].hint).toContain("org_membership_level', value: 'owner'");
+  });
+
+  it('flags a node routed only to groups (all position/team/department)', () => {
+    const findings = validateApprovalApprovers(stackWithApprovers([
+      { type: 'position', value: 'finance' },
+      { type: 'team', value: 't1' },
+      { type: 'department', value: 'bu_sales' },
+    ]));
+    expect(findings.filter(f => f.rule === APPROVAL_APPROVERS_MAY_RESOLVE_EMPTY)).toHaveLength(1);
+  });
+
+  it('does NOT flag when a guaranteed-staffed or individual fallback is present', () => {
+    // position + owner tier — the prescribed fallback.
+    expect(validateApprovalApprovers(stackWithApprovers([
+      { type: 'position', value: 'exec' },
+      { type: 'org_membership_level', value: 'owner' },
+    ]))).toEqual([]);
+    // position + a specific user.
+    expect(validateApprovalApprovers(stackWithApprovers([
+      { type: 'position', value: 'exec' },
+      { type: 'user', value: 'u1' },
+    ]))).toEqual([]);
+    // position + the submitter's manager.
+    expect(validateApprovalApprovers(stackWithApprovers([
+      { type: 'position', value: 'exec' },
+      { type: 'manager' },
+    ]))).toEqual([]);
+  });
+
+  it('drops the record-lock clause when lockRecord is false', () => {
+    const stack = stackWithApprovers([{ type: 'position', value: 'exec' }]);
+    (stack.flows as any)[0].nodes[1].config.lockRecord = false;
+    const findings = validateApprovalApprovers(stack);
+    expect(findings).toHaveLength(1);
+    expect(findings[0].rule).toBe(APPROVAL_APPROVERS_MAY_RESOLVE_EMPTY);
+    expect(findings[0].message).not.toContain('locked');
+  });
+
   it('only scans approval nodes and tolerates malformed shapes', () => {
     const findings = validateApprovalApprovers({
       flows: [{

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

@@ -20,6 +20,7 @@
  * | approval-approver-type-deprecated          | warning  | ADR-0090 D3 (#3133)        |
  * | approval-approver-type-unknown             | warning  | contract-first (PD #12)    |
  * | approval-escalation-reassign-no-target     | warning  | silent notify degradation  |
+ * | approval-approvers-may-resolve-empty       | info     | empty-position dead-end (#3424) |
  *
  * The first two are mutually exclusive by construction — a bad *value* wins,
  * because its fix (`position`) differs from the deprecation's fix
@@ -44,6 +45,19 @@ export const APPROVAL_APPROVER_NOT_MEMBERSHIP_TIER = 'approval-approver-not-memb
 export const APPROVAL_APPROVER_TYPE_DEPRECATED = 'approval-approver-type-deprecated';
 export const APPROVAL_APPROVER_TYPE_UNKNOWN = 'approval-approver-type-unknown';
 export const APPROVAL_ESCALATION_REASSIGN_NO_TARGET = 'approval-escalation-reassign-no-target';
+export const APPROVAL_APPROVERS_MAY_RESOLVE_EMPTY = 'approval-approvers-may-resolve-empty';
+
+/**
+ * Approver types that route to a GROUP whose membership is runtime data and can
+ * be empty (an unstaffed position, an empty team/department). When EVERY
+ * approver on a node is one of these, the node can resolve to an empty slate at
+ * runtime — the framework#3424 dead-end. Individually-routed types
+ * (`user`/`field`/`manager`), the guaranteed-staffed `org_membership_level`
+ * tiers, and the opaque `queue` are deliberately excluded: any of them present
+ * signals the author has a non-group route, so the node isn't purely
+ * group-gated.
+ */
+const GROUP_ROUTED_TYPES = new Set(['position', 'team', 'department']);
 
 export type ApprovalApproverSeverity = 'error' | 'warning' | 'info';
 
@@ -171,6 +185,38 @@ export function validateApprovalApprovers(stack: AnyRec): ApprovalApproverFindin
         }
       }
 
+      // Empty-slate dead-end (#3424): when EVERY approver on the node routes to
+      // a group whose membership can be empty (an unstaffed position, an empty
+      // team/department), the request can resolve to an empty `pending_approvers`
+      // at runtime — no concrete user can act, and with `lockRecord` the record
+      // stays locked with no recovery except a platform/tenant admin override.
+      // Advisory (`info`): staffing is runtime data a linter can't see, so this
+      // flags the risky SHAPE and prescribes a guaranteed-staffed fallback.
+      const routable = approvers.filter(
+        (a) => a && typeof a === 'object' && typeof (a as AnyRec).type === 'string',
+      );
+      if (
+        routable.length > 0 &&
+        routable.every((a) => GROUP_ROUTED_TYPES.has(canonicalApproverType(String((a as AnyRec).type))))
+      ) {
+        const locks = (cfg as AnyRec).lockRecord !== false; // default true
+        findings.push({
+          severity: 'info',
+          rule: APPROVAL_APPROVERS_MAY_RESOLVE_EMPTY,
+          where,
+          path: `flows[${fi}].nodes[${ni}].config.approvers`,
+          message:
+            `every approver on this node routes to a group (position/team/department) whose ` +
+            `members are runtime data — if none is staffed, the request resolves to an empty ` +
+            `slate and waits forever` +
+            (locks ? `, and (lockRecord) the record stays locked with no in-product recovery.` : `.`),
+          hint:
+            `Make sure at least one target is always staffed, or add a guaranteed-staffed ` +
+            `fallback approver, e.g. { type: 'org_membership_level', value: 'owner' }. A request ` +
+            `that still lands empty is recoverable only by a platform/tenant admin override (#3424).`,
+        });
+      }
+
       // escalation.action 'reassign' with no escalateTo silently degrades to a
       // plain SLA-breach notification at runtime — the hand-off the author
       // asked for never happens.

packages/plugins/plugin-approvals/src/approval-service.test.ts

@@ -418,11 +418,11 @@ describe('ApprovalService (node era)', () => {
   it('getRequest: viewer.can_act is true for a pending approver, false for the submitter', async () => {
     const req = await svc.openNodeRequest(openInput(['u9']), CTX); // submitter u1, approver u9
     const asApprover = await svc.getRequest(req.id, { userId: 'u9', tenantId: 't1' } as any);
-    expect(asApprover!.viewer).toEqual({ can_act: true, is_submitter: false });
+    expect(asApprover!.viewer).toEqual({ can_act: true, is_submitter: false, can_override: false });
     const asSubmitter = await svc.getRequest(req.id, { userId: 'u1', tenantId: 't1' } as any);
-    expect(asSubmitter!.viewer).toEqual({ can_act: false, is_submitter: true });
+    expect(asSubmitter!.viewer).toEqual({ can_act: false, is_submitter: true, can_override: false });
     const asOther = await svc.getRequest(req.id, { userId: 'u_stranger', tenantId: 't1' } as any);
-    expect(asOther!.viewer).toEqual({ can_act: false, is_submitter: false });
+    expect(asOther!.viewer).toEqual({ can_act: false, is_submitter: false, can_override: false });
   });
 
   it('getRequest: viewer.can_act drops to false once the request is finalized', async () => {
@@ -1034,6 +1034,117 @@ describe('ApprovalService (node era)', () => {
   });
 });
 
+// ── Admin / privileged override (#3424) ──────────────────────────────
+//
+// An approval routed to a position/team with NO holders resolves to only the
+// unresolvable `position:<name>` literal — no concrete user is in the slate, so
+// every normal decision is FORBIDDEN and (with lockRecord) the record stays
+// locked forever with no in-product recovery. A platform or tenant admin may
+// act on the pending request to release it: approve, reject, reassign it to a
+// real approver, or recall it. Privilege is org-scoped for tenant admins.
+describe('ApprovalService — admin override (#3424)', () => {
+  let engine: ReturnType<typeof makeFakeEngine>;
+  let svc: ApprovalService;
+  let n = 0;
+  const baseTime = new Date('2026-01-15T10:00:00Z').getTime();
+
+  // Admin exec contexts, shaped like the resolved authz envelope (permissions
+  // carry the permission-set names the shared resolver aggregates).
+  const PLATFORM_ADMIN = { userId: 'root', tenantId: 't1', positions: [], permissions: ['admin_full_access'] } as any;
+  const TENANT_ADMIN = { userId: 'owner', tenantId: 't1', positions: [], permissions: ['organization_admin'] } as any;
+  const OTHER_TENANT_ADMIN = { userId: 'owner2', tenantId: 't2', positions: [], permissions: ['organization_admin'] } as any;
+  const MEMBER = { userId: 'nobody', tenantId: 't1', positions: [], permissions: [] } as any;
+
+  // A request routed to an UNSTAFFED position → `pending_approvers` falls back to
+  // the `position:sales_manager` literal, undecidable by any normal user.
+  const stuckInput = (extra: Record<string, any> = {}) => ({
+    object: 'opportunity', recordId: 'opp1', runId: 'run_1', nodeId: 'approve_step',
+    flowName: 'deal_approval',
+    config: { approvers: [{ type: 'position' as const, value: 'sales_manager' }], behavior: 'first_response' as const, lockRecord: true },
+    record: { id: 'opp1', amount: 100 },
+    ...extra,
+  });
+
+  beforeEach(() => {
+    engine = makeFakeEngine();
+    n = 0;
+    svc = new ApprovalService({ engine: engine as any, clock: { now: () => new Date(baseTime + (n++) * 1000) } });
+  });
+
+  it('the stuck request is undecidable by any normal user (repro)', async () => {
+    const req = await svc.openNodeRequest(stuckInput(), CTX);
+    expect(req.pending_approvers).toEqual(['position:sales_manager']);
+    // Even the org owner-by-id is not in the resolved (empty) slate.
+    await expect(svc.decideNode(req.id, { decision: 'approve', actorId: 'nobody' }, MEMBER))
+      .rejects.toThrow(/FORBIDDEN/);
+  });
+
+  it('a tenant admin can approve a stuck request, finalizing it (which releases the lock)', async () => {
+    const req = await svc.openNodeRequest(stuckInput(), CTX);
+    const out = await svc.decide(req.id, { decision: 'approve', actorId: 'owner' }, TENANT_ADMIN);
+    expect(out.finalized).toBe(true);
+    expect(out.request.status).toBe('approved');
+    // No pending request remains → the record-lock hook no longer blocks edits.
+    const fresh = await svc.getRequest(req.id, SYS);
+    expect(fresh?.status).toBe('approved');
+    expect(fresh?.pending_approvers).toEqual([]);
+    // Audited under the admin's own id — never spoofed as an approver.
+    const acts = await svc.listActions(req.id, SYS);
+    expect(acts.at(-1)).toMatchObject({ action: 'approve', actor_id: 'owner' });
+  });
+
+  it('a platform admin can reject a stuck request', async () => {
+    const req = await svc.openNodeRequest(stuckInput(), CTX);
+    const out = await svc.decide(req.id, { decision: 'reject', actorId: 'root' }, PLATFORM_ADMIN);
+    expect(out.finalized).toBe(true);
+    expect(out.request.status).toBe('rejected');
+  });
+
+  it('an admin override finalizes even a unanimous request immediately (not one vote among the slate)', async () => {
+    const req = await svc.openNodeRequest(stuckInput({
+      config: { approvers: [{ type: 'position' as const, value: 'sales_manager' }], behavior: 'unanimous' as const, lockRecord: true },
+    }), CTX);
+    const out = await svc.decide(req.id, { decision: 'approve', actorId: 'owner' }, TENANT_ADMIN);
+    expect(out.finalized).toBe(true);
+    expect(out.request.status).toBe('approved');
+  });
+
+  it('an admin can reassign a stuck request to a real approver, who then decides normally', async () => {
+    const req = await svc.openNodeRequest(stuckInput(), CTX);
+    const out = await svc.reassign(req.id, { actorId: 'owner', to: 'u7' }, TENANT_ADMIN);
+    expect(out.request.pending_approvers).toEqual(['u7']);
+    const decided = await svc.decideNode(
+      req.id, { decision: 'approve', actorId: 'u7' },
+      { userId: 'u7', tenantId: 't1', positions: [], permissions: [] } as any,
+    );
+    expect(decided.finalized).toBe(true);
+  });
+
+  it('an admin can recall (withdraw) a stuck request', async () => {
+    const req = await svc.openNodeRequest(stuckInput(), CTX);
+    const out = await svc.recall(req.id, { actorId: 'owner', comment: 'unstaffed role' }, TENANT_ADMIN);
+    expect(out.request.status).toBe('recalled');
+    expect(out.request.pending_approvers).toEqual([]);
+  });
+
+  it('a tenant admin of a DIFFERENT org cannot override (privilege is org-scoped)', async () => {
+    const req = await svc.openNodeRequest(stuckInput(), CTX); // organization_id = t1
+    await expect(svc.decideNode(req.id, { decision: 'approve', actorId: 'owner2' }, OTHER_TENANT_ADMIN))
+      .rejects.toThrow(/FORBIDDEN/);
+  });
+
+  it('viewer.can_override reflects the privilege, and drops once finalized', async () => {
+    const req = await svc.openNodeRequest(stuckInput(), CTX);
+    const asAdmin = await svc.getRequest(req.id, TENANT_ADMIN);
+    expect(asAdmin!.viewer).toMatchObject({ can_act: false, can_override: true });
+    const asMember = await svc.getRequest(req.id, MEMBER);
+    expect(asMember!.viewer!.can_override).toBe(false);
+    await svc.decide(req.id, { decision: 'approve', actorId: 'owner' }, TENANT_ADMIN);
+    const after = await svc.getRequest(req.id, TENANT_ADMIN);
+    expect(after!.viewer!.can_override).toBe(false);
+  });
+});
+
 describe('record-lock hook (node era)', () => {
   let engine: ReturnType<typeof makeFakeEngine>;
   let svc: ApprovalService;