@@ -0,0 +1,20 @@
+---
+'@objectstack/plugin-approvals': patch
+---
+
+fix(plugin-approvals): a `manager` approver whose record owner has no manager adds no slot to the request, instead of the literal `manager:undefined`
+
+Clause-②: no
+
+An approval request whose user-resolving approver resolves to nobody now opens on an empty `pending_approvers`, and the node's `onEmptyApprovers` policy applies, as `content/docs/automation/approvals.mdx` documents ("An entry that resolves to nobody is not an error: the request opens with an empty `pending_approvers`"). This covers `{ type: 'manager' }` when the record's owner has no `sys_user.manager_id`, or the manager is outside the request's organization, and `{ type: 'field' }` when there is no record to read.
+
+Before this fix the expansion stored a literal `manager:<value>` slot: `manager:undefined` when the approver authors no `value`, which is the usual case. Both read doors (`GET /api/v1/approvals/requests` and `GET /api/v1/data/sys_approval_request`) served that slot. Nobody can ever act under it: a caller acts only under their user id, their own account's email, or the `position:<name>` address of a position they hold.
+
+What changes, and for whom:
+
+- **A slate made only of such rungs** opens with `pending_approvers: []` under the default `onEmptyApprovers: 'admin_rescue'`, where it held `["manager:undefined"]` before. `fail`, `auto_approve` and `fallback` behave as before, because those policies already treated a slate of literals as empty.
+- **A slate with other approvers** (for example `[{ type: 'manager' }, { type: 'user', value: 'u9' }]`) is carried by the others. Under `unanimous`, `quorum` or `per_group`, the dead literal used to be an unapprovable slot: it held the request open after every real approver had approved, until an admin overrode it. Now the real approvers' approvals finish the step. This is the "second approver entry that cannot resolve empty" escape that the docs name for an unset manager.
+- **Group approvers are unchanged.** An unstaffed `position` still opens on its `position:<name>` literal, which a holder staffed into the position later decides. `team`, `department` and `org_membership_level` still fall back to their `type:value` literal.
+- The `manager` rung still logs a warning when it resolves to nobody. The warning now says that the rung adds no slot, and names the subject user.
+
+Stored requests are not rewritten. A pending request that already holds a `manager:…` or `field:…` slot keeps it, and it stays recoverable as before: a platform or tenant admin can approve, reject, reassign or recall it. To list such requests, filter `sys_approval_request` on `status = pending` and on `pending_approvers` containing `manager:` or `field:`.
@@ -3349,17 +3349,15 @@ describe('ApprovalService — a graph approver that expands to nobody warns (#38
     },
   });
 
+  // `manager` is no longer a row here (#22558): it names a PERSON, so resolving
+  // to nobody now adds no slot at all instead of the dead `manager:undefined`
+  // literal this table used to pin — see the `manager` case just below and the
+  // #22558 describe further down.
   it.each([
     ['team', 'team_gone'],
     ['department', 'bu_gone'],
     ['position', 'nobody_holds_this'],
     ['org_membership_level', 'member'],
-    // `manager` is in GRAPH_APPROVER_TYPES like the four above, and the row was
-    // missing from this table. It is the worst of the set, not the mildest: it
-    // authors no `value`, so the dead literal it leaves is `manager:undefined`
-    // — a slot that names a person who does not exist AND reads like a bug in
-    // the platform to whoever finds it in `pending_approvers`.
-    ['manager', undefined],
   ])('%s: the dead literal is logged with its type, value and org', async (type, value) => {
     const engine = makeFakeEngine();
     const { svc, warnings } = svcWithWarnings(engine);
@@ -3372,6 +3370,21 @@ describe('ApprovalService — a graph approver that expands to nobody warns (#38
     expect(hit[1]).toMatchObject({ type, value, organizationId: 't1' });
   });
 
+  // The warning `manager` had as a graph type survives without a slot: an
+  // unset `sys_user.manager_id` is the common cause and nothing else on the
+  // request names it. What it must no longer say is that a slot routes anywhere.
+  it('manager: resolving to nobody adds NO slot, and the warning says so', async () => {
+    const engine = makeFakeEngine();
+    const { svc, warnings } = svcWithWarnings(engine);
+    const req = await svc.openNodeRequest(approverInput('manager') as any, CTX) as any;
+
+    expect(req.pending_approvers).toEqual([]);
+    expect(warnings.filter(([msg]) => String(msg).includes('expanded to nobody'))).toEqual([]);
+    const hit = warnings.find(([msg]) => String(msg).includes("approver 'manager' resolved to nobody"));
+    expect(hit, 'no warning for manager').toBeTruthy();
+    expect(hit[1]).toMatchObject({ type: 'manager', value: null, organizationId: 't1' });
+  });
+
   it('stays quiet when the graph DOES resolve someone', async () => {
     const engine = makeFakeEngine();
     engine._tables['sys_team_member'] = [{ id: 'tm1', team_id: 'team_ok', user_id: 'u5' }];
@@ -3394,13 +3407,15 @@ describe('ApprovalService — a graph approver that expands to nobody warns (#38
 
 // The empty-slate policy that NAMES people. Before it, an empty
 // `{ type: 'manager' }` rung had three endings and all three were bad: open on
-// `manager:undefined` and wait for an admin (`admin_rescue`), kill the run
-// (`fail`), or wave the record through (`auto_approve`). `fallback` adds the
+// a slate nobody can act on and wait for an admin (`admin_rescue`), kill the
+// run (`fail`), or wave the record through (`auto_approve`). `fallback` adds the
 // fourth — open on someone who can actually decide.
 //
-// The rung is not special. Every graph approver type ends at the same literal;
-// `manager` is just the one that reaches it without anybody authoring a wrong
-// value, which is why it is the proving case here.
+// The rung is not special: every approver type can resolve to nobody. `manager`
+// is just the one that gets there without anybody authoring a wrong value,
+// which is why it is the proving case here. Since #22558 it leaves an EMPTY
+// slate rather than the dead `manager:undefined` literal, so the
+// `admin_rescue` degradations below open on `[]`.
 describe("ApprovalService — onEmptyApprovers: 'fallback' (node-level named rescue)", () => {
   const svcWithWarnings = (engine: any) => {
     const warnings: any[] = [];
@@ -3427,6 +3442,11 @@ describe("ApprovalService — onEmptyApprovers: 'fallback' (node-level named res
   const OWNER_BACKSTOP = { type: 'user' as const, value: 'u_backstop' };
   const said = (warnings: any[], needle: string) =>
     warnings.some(([msg]) => String(msg).includes(needle));
+  // The fallback-FIRED warning. Not the bare policy spelling: the empty
+  // `manager` rung's own warning names `onEmptyApprovers: 'fallback'` as its
+  // remedy (#22558), so that needle would read "the fallback fired" off a
+  // warning that only recommends it.
+  const FALLBACK_FIRED = "per onEmptyApprovers: 'fallback'";
 
   it('opens the request on the declared fallback, never on the manager:undefined literal', async () => {
     const engine = makeFakeEngine();
@@ -3439,7 +3459,7 @@ describe("ApprovalService — onEmptyApprovers: 'fallback' (node-level named res
     expect(req.status).toBe('pending');
     expect(req.pending_approvers).toEqual(['u_backstop']);
     expect(req.pending_approvers).not.toContain('manager:undefined');
-    expect(said(warnings, "onEmptyApprovers: 'fallback'")).toBe(true);
+    expect(said(warnings, FALLBACK_FIRED)).toBe(true);
     // The request is decidable by a person, so the admin-takeover warning must
     // NOT fire — it would be false about this request.
     expect(said(warnings, 'decidable only by a privileged admin')).toBe(false);
@@ -3493,7 +3513,8 @@ describe("ApprovalService — onEmptyApprovers: 'fallback' (node-level named res
     }), CTX) as any;
 
     expect(req.status).toBe('pending');
-    expect(req.pending_approvers).toEqual(['manager:undefined']);
+    // The abandoned primary slate: the empty manager rung added no slot.
+    expect(req.pending_approvers).toEqual([]);
     expect(said(warnings, 'resolved to nobody either')).toBe(true);
     expect(said(warnings, 'decidable only by a privileged admin')).toBe(true);
   });
@@ -3506,7 +3527,7 @@ describe("ApprovalService — onEmptyApprovers: 'fallback' (node-level named res
     const req = await svc.openNodeRequest(managerNode({ onEmptyApprovers: 'fallback' }), CTX) as any;
 
     expect(req.status).toBe('pending');
-    expect(req.pending_approvers).toEqual(['manager:undefined']);
+    expect(req.pending_approvers).toEqual([]);
     expect(said(warnings, 'with no fallbackApprovers')).toBe(true);
     expect(said(warnings, 'decidable only by a privileged admin')).toBe(true);
   });
@@ -3526,7 +3547,7 @@ describe("ApprovalService — onEmptyApprovers: 'fallback' (node-level named res
     } as any, CTX) as any;
 
     expect(req.pending_approvers).toEqual(['u9']);
-    expect(said(warnings, "onEmptyApprovers: 'fallback'")).toBe(false);
+    expect(said(warnings, FALLBACK_FIRED)).toBe(false);
   });
 
   // ── negative controls ─────────────────────────────────────────────
@@ -3554,15 +3575,198 @@ describe("ApprovalService — onEmptyApprovers: 'fallback' (node-level named res
     expect(engine._tables['sys_approval_request'] ?? []).toHaveLength(0);
   });
 
-  it("'admin_rescue' still opens on the dead literal and warns", async () => {
+  it("'admin_rescue' still opens — on the empty slate — and warns", async () => {
     const engine = makeFakeEngine();
     const { svc, warnings } = svcWithWarnings(engine);
     const req = await svc.openNodeRequest(managerNode({
       onEmptyApprovers: 'admin_rescue', fallbackApprovers: [OWNER_BACKSTOP],
     }), CTX) as any;
-    expect(req.pending_approvers).toEqual(['manager:undefined']);
+    expect(req.pending_approvers).toEqual([]);
+    expect(said(warnings, 'decidable only by a privileged admin')).toBe(true);
+    expect(said(warnings, FALLBACK_FIRED)).toBe(false);
+  });
+});
+
+// #22558 — a PERSON approver (`manager`, `field`) that resolves to nobody adds
+// NO slot. Measured on 17.7.0 in a downstream app: a `{ type: 'manager' }` step
+// for a record whose owner carries no `manager_id` opened with
+// `pending_approvers: ["manager:undefined"]` on both read doors — a slot that
+// reads as real, interpolates a missing value, and can be taken by nobody (the
+// acting path admits only a user id, the caller's own email, or a held
+// `position:<p>` address). The documented contract
+// (`content/docs/automation/approvals.mdx`) is that an entry resolving to
+// nobody contributes nothing and the node's `onEmptyApprovers` policy decides
+// the empty slate.
+//
+// The directory is REAL here, not absent: the owner has a `sys_user` row whose
+// `manager_id` is null — the exact shape the report measured — beside a second
+// owner who does have one (the control).
+describe('ApprovalService — a person approver that resolves to nobody adds no slot (#22558)', () => {
+  const svcWithWarnings = (engine: any) => {
+    const warnings: any[] = [];
+    let n = 0;
+    const svc = new ApprovalService({
+      engine,
+      clock: { now: () => new Date(1757000000000 + (n++) * 1000) },
+      logger: { warn: (msg: any, meta: any) => warnings.push([msg, meta]) },
+    });
+    return { svc, warnings };
+  };
+  const said = (warnings: any[], needle: string) =>
+    warnings.some(([msg]) => String(msg).includes(needle));
+
+  const seedDirectory = (engine: any) => {
+    engine._tables['sys_user'] = [
+      { id: 'u_no_mgr', manager_id: null },
+      { id: 'u_has_mgr', manager_id: 'u_boss' },
+      { id: 'u_boss', manager_id: null },
+    ];
+  };
+
+  /** A node on a record owned by `owner`, with the given approvers / config. */
+  const node = (owner: string, approvers: any[], configExtra: Record<string, any> = {}) => ({
+    ...openInput([]),
+    record: { id: 'opp1', amount: 100, owner_id: owner },
+    config: {
+      approvers,
+      behavior: 'first_response' as const,
+      lockRecord: false,
+      ...configExtra,
+    },
+  });
+
+  it('a manager step for an owner with no manager_id opens on an EMPTY pending_approvers, and admin_rescue fires', async () => {
+    const engine = makeFakeEngine();
+    seedDirectory(engine);
+    const { svc, warnings } = svcWithWarnings(engine);
+    const req = await svc.openNodeRequest(node('u_no_mgr', [{ type: 'manager' }]), CTX) as any;
+
+    expect(req.status).toBe('pending');
+    expect(req.pending_approvers).toEqual([]);
+    // The stored row — what both read doors serve — holds no slot either.
+    const stored = String(engine._tables['sys_approval_request'][0].pending_approvers ?? '');
+    expect(stored.split(',').filter(Boolean)).toEqual([]);
+    // `onEmptyApprovers` defaults to admin_rescue, and it fired on this slate.
     expect(said(warnings, 'decidable only by a privileged admin')).toBe(true);
-    expect(said(warnings, "onEmptyApprovers: 'fallback'")).toBe(false);
+  });
+
+  it("the same step under onEmptyApprovers: 'fallback' opens on the named people", async () => {
+    const engine = makeFakeEngine();
+    seedDirectory(engine);
+    const { svc } = svcWithWarnings(engine);
+    const req = await svc.openNodeRequest(node('u_no_mgr', [{ type: 'manager' }], {
+      onEmptyApprovers: 'fallback',
+      fallbackApprovers: [{ type: 'user', value: 'u_backstop' }],
+    }), CTX) as any;
+
+    expect(req.pending_approvers).toEqual(['u_backstop']);
+  });
+
+  it("control: an owner WITH a manager gets that manager's slot", async () => {
+    const engine = makeFakeEngine();
+    seedDirectory(engine);
+    const { svc, warnings } = svcWithWarnings(engine);
+    const req = await svc.openNodeRequest(node('u_has_mgr', [{ type: 'manager' }]), CTX) as any;
+
+    expect(req.pending_approvers).toEqual(['u_boss']);
+    expect(said(warnings, "approver 'manager' resolved to nobody")).toBe(false);
+    expect(said(warnings, 'decidable only by a privileged admin')).toBe(false);
+  });
+
+  // Beside other entries an empty manager rung contributes nothing, so the
+  // others carry the request — the "second entry that cannot resolve empty"
+  // escape the docs name for an unset manager. Under `unanimous` the dead
+  // literal used to be an unapprovable slot that held the request open after
+  // every real approver had approved.
+  it('beside a concrete approver, an empty manager rung adds nothing and does not hold a unanimous request open', async () => {
+    const engine = makeFakeEngine();
+    seedDirectory(engine);
+    const { svc } = svcWithWarnings(engine);
+    const req = await svc.openNodeRequest(node(
+      'u_no_mgr', [{ type: 'manager' }, { type: 'user', value: 'u9' }], { behavior: 'unanimous' },
+    ), CTX) as any;
+    expect(req.pending_approvers).toEqual(['u9']);
+
+    const out = await svc.decideNode(req.id, { decision: 'approve', actorId: 'u9' }, SYS);
+    expect(out.finalized).toBe(true);
+    expect(out.request.status).toBe('approved');
+  });
+
+  // `field` is the other person type. On the open path the record is always
+  // present (the live re-read falls back to the trigger snapshot, then `{}`),
+  // so an empty field already contributed nothing; this pins that it still
+  // does, beside the manager change.
+  it('an empty field approver adds no slot either', async () => {
+    const engine = makeFakeEngine();
+    const { svc } = svcWithWarnings(engine);
+    const req = await svc.openNodeRequest(node('u_no_mgr', [{ type: 'field', value: 'reviewer' }]), CTX) as any;
+    expect(req.pending_approvers).toEqual([]);
+  });
+
+  // The kept half: a GROUP type's literal is untouched. For `position` the
+  // literal IS an acting address — a holder staffed in later decides it.
+  it('kept: an unstaffed position still opens on its position literal', async () => {
+    const engine = makeFakeEngine();
+    const { svc } = svcWithWarnings(engine);
+    const req = await svc.openNodeRequest(node('u_no_mgr', [{ type: 'position', value: 'cfo' }]), CTX) as any;
+    expect(req.pending_approvers).toEqual(['position:cfo']);
+  });
+
+  // ── stored rows: the disposition of a request opened before this fix ──
+  //
+  // A pending request a pre-fix release opened already holds the dead slot.
+  // Nothing rewrites it. Each pin below takes a request the current code
+  // opened on `[manager, user u9]` and puts the STORED row back into the
+  // pre-fix shape, then drives its next transition.
+
+  /** Open `[manager, user u9]` unanimous, then hand back the stored row. */
+  const openUnanimous = async (svc: ApprovalService, engine: any) => {
+    const req = await svc.openNodeRequest(node(
+      'u_no_mgr', [{ type: 'manager' }, { type: 'field', value: 'reviewer' }, { type: 'user', value: 'u9' }],
+      { behavior: 'unanimous' },
+    ), CTX) as any;
+    return { req, row: engine._tables['sys_approval_request'][0] };
+  };
+
+  it('stored row WITH the open-time slate snapshot keeps its dead slot: not rewritten, still an admin rescue', async () => {
+    const engine = makeFakeEngine();
+    seedDirectory(engine);
+    const { svc } = svcWithWarnings(engine);
+    const { req, row } = await openUnanimous(svc, engine);
+    // The pre-fix shape: the literal on the slate AND in the tally snapshot.
+    const cfg = JSON.parse(row.node_config_json);
+    cfg.__approverGroups = { 'manager:undefined': ['#0'], u9: ['#2'] };
+    row.node_config_json = JSON.stringify(cfg);
+    row.pending_approvers = 'manager:undefined,u9';
+
+    const vote = await svc.decideNode(req.id, { decision: 'approve', actorId: 'u9' }, SYS);
+    expect(vote.finalized).toBe(false);
+    expect(vote.request.pending_approvers).toEqual(['manager:undefined']);
+
+    // The recovery it always had: a privileged override decides it outright.
+    const rescue = await svc.decideNode(req.id, { decision: 'approve', actorId: 'admin_x' }, SYS);
+    expect(rescue.finalized).toBe(true);
+    expect(rescue.request.status).toBe('approved');
+  });
+
+  // A request opened before the tally snapshot existed is re-resolved at its
+  // next approve, against the stored payload — none here, so the person
+  // rungs have no record to read (the one way `field` reaches the no-slot
+  // branch). Both now add nothing, so the dead slots stop counting.
+  it('stored row WITHOUT a snapshot is re-evaluated at its next approve: the dead person slots stop counting', async () => {
+    const engine = makeFakeEngine();
+    seedDirectory(engine);
+    const { svc } = svcWithWarnings(engine);
+    const { req, row } = await openUnanimous(svc, engine);
+    const cfg = JSON.parse(row.node_config_json);
+    delete cfg.__approverGroups;
+    row.node_config_json = JSON.stringify(cfg);
+    row.payload_json = null;
+    row.pending_approvers = 'manager:undefined,u9';
+
+    const vote = await svc.decideNode(req.id, { decision: 'approve', actorId: 'u9' }, SYS);
+    expect(vote.finalized).toBe(true);
+    expect(vote.request.status).toBe('approved');
   });
 });