fix(approvals): actor_id records the person who decided, acted_as the slot it was taken as; stored slot literals move out at boot

.changeset/21411-approval-actor-person.md

@@ -0,0 +1,27 @@
+---
+'@objectstack/plugin-approvals': patch
+---
+
+An approval action now records the user who took it in `sys_approval_action.actor_id`, and the pending-approver slot it was taken as in a new `acted_as` column; rows stored before this move their slot out of `actor_id` at the next boot
+
+Clause-②: no
+
+`actor_id` is a lookup to `sys_user`, so under ADR-0118 D1 it holds a user id or nothing. A slot-gated action used to record the slot it took there instead: a `position:<name>` literal for a position staffed after the request opened, or an email for a `user` approver authored as one. On those decisions no record named the person who decided. The audit ledger and activity rows the write produces carry no user, so the attribution was lost, and every join or report on the lookup silently dropped the row.
+
+**This supersedes the "What is recorded" sentence of the unreleased `21379-position-address-readers` changeset**, which says `actor_id` holds the slot. From this release it holds the person.
+
+- **What is recorded.**
+  - `actor_id` is the user the request's context vouches for: the signed-in caller, whatever address they named.
+  - `acted_as` is the slot the action took, in the slot's stored spelling (a user id, an email, or `position:<name>`). It is empty on actions no slot admitted: the submitter's own actions, system actions, and an admin override, which `via_override` still marks.
+  - An emailed action link records the one account that carries the token's email. If no account carries it, the link records no person.
+  - The SLA sweep keeps its reserved `system:sla` actor for now.
+- **What reads it.**
+  - The multi-approver tally and `decision_progress` count `acted_as`.
+  - A participant who already acted keeps sight of a request by either of two facts: `actor_id` is their user id, or `acted_as` is a slot they act under (so a decision taken as `position:<name>` stays visible to that position's holders).
+  - Nothing compares a slot with `actor_id` any more.
+  - The action log (`GET /api/v1/approvals/requests/:id/actions`, `listActions`) returns `acted_as` beside `actor_id` and `actor_name`, filling the `ApprovalActionRow.acted_as` member `@objectstack/spec` declares. It is omitted when the action took no slot, or when no stored record kept the slot.
+- **Stored rows.** A repair runs on every boot and is idempotent.
+  - Pass 1: a row whose `actor_id` still holds a slot address gets `acted_as` set to it and `actor_id` cleared. No stored record names who decided it, so it shows the slot and no person.
+  - Pass 2: the approve votes a still-pending request's tally counts get their `acted_as`, so in-flight `unanimous`, `quorum` and `per_group` requests keep the approvals they already collected.
+  - A failure is logged at error level and retried at the next boot.
+- **For a report or integration that read `actor_id` as the slot:** read `acted_as` instead. `actor_id` now always joins to `sys_user`.

content/docs/automation/approvals.mdx

@@ -509,9 +509,10 @@ 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
 not a pending approver`); a request that isn't pending returns 409
-(`INVALID_STATE`). The decision is recorded in `sys_approval_action.actor_id`
-under the slot it took, in that slot's stored spelling — the multi-approver
-tally counts approvals by matching that value against the slate. Always go through
+(`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
+took, in that slot's stored spelling — the multi-approver tally counts approvals
+by matching `acted_as` against the slate. Always go through
 these endpoints — never resume the flow run directly, and since #3801 you
 **cannot**: `POST /api/v1/automation/{flow}/runs/{runId}/resume` answers 403 for
 a run parked on an `approval` node (including via a `subflow` pause) and changes

content/docs/permissions/tenant-audit-census.mdx

@@ -122,7 +122,7 @@ are reported as `undecidable` rather than assumed either way.
 
 The same holds twice over for the context. An options argument spelled as a
 literal can be read; one spelled `options`, `{ ...opts }`, or handed through a
-forwarding shim cannot, and **67 of the 229 sites are spelled that way**. A
+forwarding shim cannot, and **67 of the 231 sites are spelled that way**. A
 context resolved from an inline literal or a local `const` can be tested for
 `isSystem`; one arriving from a helper call cannot.
 
@@ -187,10 +187,10 @@ reproduce them. Where it disagrees, it disagrees on the page:
 
 | carried figure | where it survives | this census |
 | :--- | :--- | ---: |
-| 175 write call sites | quoted in the merged changeset | **229** |
+| 175 write call sites | quoted in the merged changeset | **231** |
 | 24 carrying no tenant context | quoted in the merged changeset | **9** provable and tenancy-enabled; **34** more whose options argument is unreadable |
-| 127 of 175 statically decidable, 48 runtime-parameter-name sites | restated on the `isSystem`-scoping card | **152 of 229** decidable, **77** undecidable |
-| 135 (77%) silenced by the `isSystem` guard before the posture gate | the lost issue body — **no surviving corroboration** | **not reproduced**: 110 decidably elevated, 0 decidably not, 102 undecidable |
+| 127 of 175 statically decidable, 48 runtime-parameter-name sites | restated on the `isSystem`-scoping card | **154 of 231** decidable, **77** undecidable |
+| 135 (77%) silenced by the `isSystem` guard before the posture gate | the lost issue body — **no surviving corroboration** | **not reproduced**: 112 decidably elevated, 0 decidably not, 102 undecidable |
 | 141 and 132, two independent re-derivations | the card that filed this work | — |
 
 **The differences are not reconciled, and deliberately so.** The old census's
@@ -207,11 +207,11 @@ would report a smaller number and would not say so.
 
 The fourth row is the one worth flagging to anyone citing it. **The 135 / 77%
 figure has no surviving corroboration anywhere in the tree.** This census reads
-110 of 229 (48%) as decidably elevated, with 102 more whose elevation is a
+112 of 231 (48%) as decidably elevated, with 102 more whose elevation is a
 run-time fact — so the claim is neither confirmed nor refuted, and the honest
 answer is that a static reading cannot settle it.
 
-⇒ **Cite `9 / 229`, and say what it is**: the sites whose options argument was
+⇒ **Cite `9 / 231`, and say what it is**: the sites whose options argument was
 READ and holds no tenant context, against a decidably tenancy-enabled object.
 That is the control's provable yield surface. ⛔ Do not cite it as "the sites
 without tenant context" — **34 further sites** have an options argument this
@@ -223,28 +223,28 @@ cannot read, and they are neither in nor out.
 
 | what | count |
 | :--- | ---: |
-| write call sites on the application surface | **229** |
-| …whose object name is statically decidable | 152 |
+| write call sites on the application surface | **231** |
+| …whose object name is statically decidable | 154 |
 | …whose object name is chosen at run time | 77 |
-| …against an object with tenancy ENABLED | 151 |
+| …against an object with tenancy ENABLED | 153 |
 | …against an object that declares tenancy off | 1 |
-| threading a tenant context | 145 |
+| threading a tenant context | 147 |
 | PROVABLY carrying none (options read, no context key) | **17** |
 | …of those, against a decidably tenancy-enabled object | **9** |
 | options argument UNREADABLE — may or may not carry one | 67 |
 | …of those, against a decidably tenancy-enabled object | 34 |
-| threading a decidably ELEVATED (`isSystem`) context | 110 |
+| threading a decidably ELEVATED (`isSystem`) context | 112 |
 | threading a context that is decidably NOT elevated | 0 |
 | threading a context whose elevation is a run-time fact | 102 |
 
 | how the instrument reached the site | count |
 | :--- | ---: |
-| receiver carried a readable engine type | 181 |
+| receiver carried a readable engine type | 183 |
 | receiver erased, placed by the object NAME | 28 |
 | receiver erased, placed by an `object: string` PARAMETER | 15 |
 | receiver erased, placed by an `UNTYPED_RECEIVERS` row | 5 |
 
-| object name spelled inline | 101 |
+| object name spelled inline | 103 |
 | object name spelled through a `const` | 51 |
 | object name is an `object: string` parameter | 17 |
 | object name is some other run-time expression | 60 |
@@ -297,13 +297,13 @@ holds still. They are required to be HERE and to say WHEN they were true;
 their values are not compared. The reasoning, and the measurement behind it,
 are in `scripts/check-tenant-audit-census.mjs`.
 
-Measured on 2026-10-02 at `c41817b12`.
+Measured on 2026-10-02 at `b668cf134`.
 
 | corpus scale (not enforced) | count |
 | :--- | ---: |
-| tracked non-test sources scanned | 599 |
-| engine-shaped types recognised | 66 |
+| tracked non-test sources scanned | 602 |
+| engine-shaped types recognised | 67 |
 | declared objects in the registry | 116 |
-| same-named calls subtracted as non-engine | 150 |
+| same-named calls subtracted as non-engine | 152 |
 
 {/* END GENERATED: tenant-audit-census */}

docs/audits/2026-08-tenant-audit-write-call-sites.counts.md

@@ -33,17 +33,17 @@ silent, and `node scripts/tenant-audit-census.mjs --write` is the resolution.
 
 | Measure | Value |
 |---|---:|
-| Write call sites | 229 |
-| Object name statically decidable | 152 |
+| Write call sites | 231 |
+| Object name statically decidable | 154 |
 | Object name chosen at run time | 77 |
-| Against a tenancy-enabled object | 151 |
+| Against a tenancy-enabled object | 153 |
 | Against an object declaring tenancy off | 1 |
-| Threading a tenant context | 145 |
+| Threading a tenant context | 147 |
 | Provably carrying none | 17 |
 | …and decidably tenancy-enabled | 9 |
 | Options argument unreadable | 67 |
 | …and decidably tenancy-enabled | 34 |
-| Threading a decidably elevated context | 110 |
+| Threading a decidably elevated context | 112 |
 | Threading a decidably non-elevated context | 0 |
 | Threading a context of undecidable elevation | 102 |
 
@@ -90,21 +90,22 @@ holds still. They are required to be HERE and to say WHEN they were true;
 their values are not compared. The reasoning, and the measurement behind it,
 are in `scripts/check-tenant-audit-census.mjs`.
 
-Measured on 2026-10-02 at `c41817b12`.
+Measured on 2026-10-02 at `b668cf134`.
 
 | corpus scale (not enforced) | count |
 | :--- | ---: |
-| tracked non-test sources scanned | 599 |
-| engine-shaped types recognised | 66 |
+| tracked non-test sources scanned | 602 |
+| engine-shaped types recognised | 67 |
 | declared objects in the registry | 116 |
-| same-named calls subtracted as non-engine | 150 |
+| same-named calls subtracted as non-engine | 152 |
 
 ## Every site
 
 | file | verb | object | tenancy | tenant context | n |
 |---|---|---|---|---|---:|
 | `packages/plugins/organizations/src/claim-org-seed-ownership.ts` | `update` | `schema.name` | undecidable | elevated | 1 |
 | `packages/plugins/organizations/src/claim-orphan-org-rows.ts` | `update` | `schema.name` | undecidable | elevated | 1 |
+| `packages/plugins/plugin-approvals/src/action-slot-backfill.ts` | `update` | `sys_approval_action` | enabled | elevated | 2 |
 | `packages/plugins/plugin-approvals/src/approval-service.ts` | `update` | `object` | undecidable | context, elevation undecidable | 1 |
 | `packages/plugins/plugin-approvals/src/approval-service.ts` | `insert` | `sys_approval_action` | enabled | elevated | 14 |
 | `packages/plugins/plugin-approvals/src/approval-service.ts` | `delete` | `sys_approval_approver` | enabled | elevated | 2 |

packages/plugins/plugin-approvals/src/action-slot-backfill-wiring.test.ts

@@ -0,0 +1,86 @@
+// Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license.
+
+/**
+ * #21411 — the action-slot backfill is WIRED: `ApprovalsServicePlugin.start()`
+ * registers it on `kernel:ready`, hands it the plugin's own engine, and logs a
+ * failure at `error`.
+ *
+ * `action-slot-backfill.integration.test.ts` proves what the repair does. This
+ * file proves it runs at all — a repair module nothing calls is code `grep`
+ * finds and production never executes, and every slot reader would then miss
+ * the rows it exists to move, silently. The module is replaced by a spy so the
+ * assertion is about the call, not about the repair.
+ */
+
+import { describe, it, expect, vi, beforeEach } from 'vitest';
+
+const backfill = vi.hoisted(() => ({
+  fn: vi.fn(async (_engine: unknown) => ({ literalsMoved: 0, votesStamped: 0 })),
+}));
+
+vi.mock('./action-slot-backfill.js', async (importOriginal) => ({
+  ...(await importOriginal<typeof import('./action-slot-backfill.js')>()),
+  backfillActionSlots: backfill.fn,
+}));
+
+import { ApprovalsServicePlugin } from './approvals-plugin.js';
+
+function fakeContext(engine: unknown) {
+  const hooks: Record<string, Array<() => Promise<void> | void>> = {};
+  const logged = { info: [] as string[], warn: [] as string[], error: [] as string[] };
+  const ctx: any = {
+    hook: (name: string, fn: () => Promise<void> | void) => { (hooks[name] ??= []).push(fn); },
+    getService: (name: string) => {
+      if (name === 'objectql') return engine;
+      throw new Error(`no service '${name}'`);
+    },
+    registerService: () => {},
+    logger: {
+      info: (msg: string) => { logged.info.push(String(msg)); },
+      warn: (msg: string) => { logged.warn.push(String(msg)); },
+      error: (msg: string) => { logged.error.push(String(msg)); },
+      debug: () => {},
+    },
+  };
+  const fire = async (name: string) => { for (const fn of hooks[name] ?? []) await fn(); };
+  return { ctx, fire, logged };
+}
+
+/**
+ * Enough engine for `start()` with `disableAutoHooks`: the service only needs
+ * an object to hold, and the one boot-time read that reaches it (the
+ * approver-index rebuild) finds nothing. The repair itself is the spy above,
+ * so nothing here writes — and a double with no write verbs makes no claim
+ * about how writes dispatch.
+ */
+function fakeEngine() {
+  return { find: async () => [] };
+}
+
+describe('the action-slot backfill is wired on kernel:ready (#21411)', () => {
+  beforeEach(() => { backfill.fn.mockClear(); });
+
+  it('runs once at kernel:ready, never before, against the plugin\'s own engine', async () => {
+    const engine = fakeEngine();
+    const { ctx, fire } = fakeContext(engine);
+    await new ApprovalsServicePlugin({ disableAutoHooks: true }).start(ctx);
+
+    expect(backfill.fn, 'not at start(): the registries are still filling').not.toHaveBeenCalled();
+    await fire('kernel:ready');
+    expect(backfill.fn).toHaveBeenCalledTimes(1);
+    expect(backfill.fn.mock.calls[0][0]).toBe(engine);
+  });
+
+  it('a failed run is logged at error, naming what stays wrong and the fix', async () => {
+    backfill.fn.mockRejectedValueOnce(new Error('driver went away'));
+    const { ctx, fire, logged } = fakeContext(fakeEngine());
+    await new ApprovalsServicePlugin({ disableAutoHooks: true }).start(ctx);
+    await fire('kernel:ready');
+
+    const line = logged.error.find((m) => m.includes('action-slot backfill failed'));
+    expect(line, `error lines: ${JSON.stringify(logged.error)}`).toBeDefined();
+    expect(line).toMatch(/multi-approver tallies/);
+    expect(line).toMatch(/restart/);
+    expect(logged.warn.some((m) => m.includes('action-slot backfill'))).toBe(false);
+  });
+});