fix(automation,objectql,spec,approvals): flow system-write audit attribution (#4366) + structured reassign hand-off (#4365)

.changeset/approval-reassign-structured-parties.md

@@ -0,0 +1,24 @@
+---
+"@objectstack/spec": minor
+"@objectstack/plugin-approvals": minor
+---
+
+feat(approvals,spec): structured reassign hand-off parties on `sys_approval_action` (#4365)
+
+A reassign's audit row used to encode "who handed the slot to whom" only inside
+a default free-text comment — `"<from_id> → <to_id>"`, two raw user ids — which
+clients could neither parse reliably nor render readably, so the approvals
+timeline showed opaque identifier soup for the single most important fact of
+the entry.
+
+- `sys_approval_action` gains `reassign_from` / `reassign_to`
+  (`lookup('sys_user')`), written by `ApprovalService.reassign()`.
+- `comment` is pure user input again: nothing is invented when the actor
+  supplies none.
+- `listActions()` resolves both parties' display names into
+  `reassign_from_name` / `reassign_to_name`, alongside the existing
+  `actor_name`, so timelines can render "from A to B" without extra lookups.
+- `ApprovalActionRow` (spec contract) declares the four new fields.
+
+Pre-existing rows keep their legacy comment; clients should prefer the
+structured fields when present and fall back to `comment` otherwise.

.changeset/flow-system-run-audit-attribution.md

@@ -0,0 +1,29 @@
+---
+"@objectstack/spec": patch
+"@objectstack/service-automation": patch
+"@objectstack/objectql": patch
+---
+
+fix(automation,objectql,spec): attribute `runAs:'system'` flow writes to the flow in the audit log (#4366)
+
+A `runAs:'system'` flow's data writes carried no attribution at all: the run
+context resolved to `{ isSystem: true }` with no `userId` and no service
+principal, so the audit writer recorded `user_id=null, actor=null` and the
+record-history UI rendered every such row as "Unknown user" — business users
+read the flow's own status mirror as data corruption.
+
+The `svc:*` attribution channel (ADR-0014 D2, `ExecutionContext.actor`) already
+existed for exactly this class of writer; it was simply never wired end-to-end:
+
+- **service-automation** — `resolveRunContext` now stamps `flowName` alongside
+  `runAs`/`flowRunId`, and `resolveRunDataContext` labels a `runAs:'system'`
+  run's data context `actor: 'svc:flow:<flowName>'` (fallback
+  `svc:flow:automation`). Attribution only — no security middleware keys on it.
+- **objectql** — `buildSession` propagates `ExecutionContext.actor` onto the
+  hook session, closing the gap that left the audit writer's
+  `userId ?? session.actor` fallback unreachable from the engine path.
+- **spec** — `AutomationContext.flowName` (engine-stamped, provenance) and the
+  hook session's optional `actor` field document the contract.
+
+No behavior change for user-attributed writes: `userId` still wins wherever it
+is present.

content/docs/references/data/hook.mdx

@@ -37,7 +37,7 @@ const result = HookContext.parse(data);
 | **input** | `Record<string, any>` | ✅ | Mutable input parameters |
 | **result** | `any` | optional | Operation result (After hooks only) |
 | **previous** | `Record<string, any>` | optional | Record state before operation |
-| **session** | `{ userId?: string; organizationId?: string; roles?: string[]; accessToken?: string; … }` | optional | Current session context |
+| **session** | `{ userId?: string; actor?: string; organizationId?: string; roles?: string[]; … }` | optional | Current session context |
 | **provenance** | `{ flowRunId?: string }` | optional | Server-stamped write provenance (never client-supplied, never an authorization input) |
 | **transaction** | `any` | optional | Database transaction handle |
 | **ql** | `any` | ✅ | ObjectQL Engine Reference |

packages/objectql/src/engine.test.ts

@@ -484,6 +484,39 @@ describe('ObjectQL Engine', () => {
         });
     });
 
+    /**
+     * ADR-0014 D2 / #4366 — the service-principal label. A non-user write (a
+     * `runAs:'system'` flow, a service token) carries `ExecutionContext.actor`;
+     * the audit writer's `userId ?? session.actor` fallback is dead unless the
+     * engine propagates it onto the hook session.
+     */
+    describe('service-principal actor propagated to the hook session (#4366)', () => {
+        beforeEach(async () => {
+            engine.registerDriver(mockDriver, true);
+            await engine.init();
+            vi.mocked(SchemaRegistry.getObject).mockReturnValue({ name: 'task', fields: {} } as any);
+        });
+
+        it('surfaces context.actor as session.actor for a system write', async () => {
+            let session: any;
+            engine.registerHook('afterInsert', async (ctx: any) => { session = ctx.session; }, { object: 'task' });
+
+            await engine.insert('task', { title: 'x' }, { context: { isSystem: true, actor: 'svc:flow:mirror_status' } as any });
+
+            expect(session).toMatchObject({ isSystem: true, actor: 'svc:flow:mirror_status' });
+        });
+
+        it('omits session.actor when the context carries none (no anonymous label invented)', async () => {
+            let session: any;
+            engine.registerHook('afterInsert', async (ctx: any) => { session = ctx.session; }, { object: 'task' });
+
+            await engine.insert('task', { title: 'y' }, { context: { userId: 'u1' } as any });
+
+            expect(session.userId).toBe('u1');
+            expect(session.actor).toBeUndefined();
+        });
+    });
+
     describe('organizationId exposed to hooks as the blessed org name (#3280)', () => {
         beforeEach(async () => {
             engine.registerDriver(mockDriver, true);

packages/objectql/src/engine.ts

@@ -858,6 +858,13 @@ export class ObjectQL implements IDataEngine {
       // Propagate system-elevated flag so hooks can distinguish engine
       // self-writes (e.g. approval status mirror) from genuine user writes.
       ...((execCtx as any).isSystem ? { isSystem: true } : {}),
+      // Propagate the service-principal label (`ExecutionContext.actor`,
+      // e.g. `svc:flow:<name>`) so a non-user write stays attributable in the
+      // audit log — the writer's `userId ?? session.actor` fallback is dead
+      // without this hop (ADR-0014 D2, #4366).
+      ...(typeof (execCtx as any).actor === 'string' && (execCtx as any).actor
+        ? { actor: (execCtx as any).actor }
+        : {}),
       // Propagate the automation-suppression flag so the record-change trigger
       // can skip flow dispatch for seed/bulk writes (ADR: seed loads end-state
       // data, not user events). `skipAutomations` implies `skipTriggers` —

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

@@ -1121,12 +1121,38 @@ describe('ApprovalService (node era)', () => {
 
   // ── thread interactions ─────────────────────────────────────────
 
-  it('reassign: hands the slot to a new approver and audits the move', async () => {
+  it('reassign: hands the slot to a new approver and audits the move on structured fields (#4365)', async () => {
     const req = await svc.openNodeRequest(openInput(['u9', 'u2']), CTX);
     const out = await svc.reassign(req.id, { actorId: 'u9', to: 'u7' }, asUser('u9'));
     expect(out.request.pending_approvers).toEqual(['u7', 'u2']);
     const actions = await svc.listActions(req.id, SYS);
-    expect(actions.at(-1)).toMatchObject({ action: 'reassign', actor_id: 'u9', comment: 'u9 → u7' });
+    const audit = actions.at(-1)!;
+    expect(audit).toMatchObject({ action: 'reassign', actor_id: 'u9', reassign_from: 'u9', reassign_to: 'u7' });
+    // No user comment → none invented. The old default baked raw user ids
+    // into user-facing text ("u9 → u7").
+    expect(audit.comment).toBeUndefined();
+  });
+
+  it('reassign: a user comment is stored verbatim alongside the structured fields (#4365)', async () => {
+    const req = await svc.openNodeRequest(openInput(['u9']), CTX);
+    await svc.reassign(req.id, { actorId: 'u9', to: 'u7', comment: 'On leave next week' }, asUser('u9'));
+    const actions = await svc.listActions(req.id, SYS);
+    expect(actions.at(-1)).toMatchObject({
+      action: 'reassign', reassign_from: 'u9', reassign_to: 'u7', comment: 'On leave next week',
+    });
+  });
+
+  it('reassign: listActions resolves the hand-off parties to display names (#4365)', async () => {
+    engine._tables['sys_user'] = [
+      { id: 'u9', name: 'Grace Hopper', email: 'grace@example.com' },
+      { id: 'u7', name: 'Ada Lovelace', email: 'ada@example.com' },
+    ];
+    const req = await svc.openNodeRequest(openInput(['u9']), CTX);
+    await svc.reassign(req.id, { actorId: 'u9', to: 'u7' }, asUser('u9'));
+    const actions = await svc.listActions(req.id, SYS);
+    expect(actions.at(-1)).toMatchObject({
+      reassign_from_name: 'Grace Hopper', reassign_to_name: 'Ada Lovelace',
+    });
   });
 
   it('reassign: notifies the new approver via messaging', async () => {

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

@@ -434,6 +434,9 @@ function rowFromAction(row: any): ApprovalActionRow {
     action: row.action,
     actor_id: row.actor_id ?? undefined,
     comment: row.comment ?? undefined,
+    // Structured reassign hand-off parties (#4365).
+    reassign_from: row.reassign_from ?? undefined,
+    reassign_to: row.reassign_to ?? undefined,
     // Decision attachments (#3266): rich descriptors carrying the display name +
     // download URL, so consumers label/open them without reading `sys_file`.
     attachments: attachments.length ? attachments : undefined,
@@ -2276,7 +2279,11 @@ export class ApprovalService implements IApprovalService {
     await this.engine.insert('sys_approval_action', {
       id: uid('aact'), request_id: requestId, organization_id: raw.organization_id ?? null,
       step_name: raw.flow_node_id ?? raw.current_step ?? null, step_index: 0, action: 'reassign',
-      actor_id: actorId, comment: input.comment ?? `${from} → ${to}`, created_at: now,
+      // The hand-off parties are STRUCTURED fields (#4365) — the old default
+      // comment (`"<from> → <to>"`) baked raw user ids into user-facing text.
+      // `comment` is pure user input: absent unless the actor wrote one.
+      actor_id: actorId, reassign_from: from, reassign_to: to,
+      comment: input.comment ?? null, created_at: now,
     }, { context: SYSTEM_CTX });
     // per_group / quorum (#3266): carry the delegated slot's group membership to
     // the new approver in the snapshot, so their approval still counts for the
@@ -3620,13 +3627,20 @@ export class ApprovalService implements IApprovalService {
     });
     const actions = Array.isArray(rows) ? rows.map(rowFromAction) : [];
     // Timeline display: resolve actor ids to names so the audit trail never
-    // shows a raw identifier. Role/team literals are already readable.
+    // shows a raw identifier. Role/team literals are already readable. The
+    // reassign hand-off parties (#4365) resolve through the same batch.
     const names = await this.resolveUserNames(
-      actions.map(a => a.actor_id).filter(id => id && !id.includes(':')),
+      actions
+        .flatMap(a => [a.actor_id, a.reassign_from, a.reassign_to])
+        .filter(id => id && !id.includes(':')),
     );
     for (const a of actions as any[]) {
       const n = a.actor_id ? names.get(String(a.actor_id)) : undefined;
       if (n) a.actor_name = n;
+      const fromName = a.reassign_from ? names.get(String(a.reassign_from)) : undefined;
+      if (fromName) a.reassign_from_name = fromName;
+      const toName = a.reassign_to ? names.get(String(a.reassign_to)) : undefined;
+      if (toName) a.reassign_to_name = toName;
     }
     return actions;
   }

packages/plugins/plugin-approvals/src/sys-approval-action.object.ts

@@ -119,6 +119,25 @@ export const SysApprovalAction = ObjectSchema.create({
 
     comment: Field.textarea({ label: 'Comment', required: false, group: 'Action' }),
 
+    // Structured hand-off parties for `action: 'reassign'` (#4365). Before
+    // these existed the pair lived only inside a default free-text comment
+    // ("<from_id> → <to_id>"), which no client could parse or render readably.
+    // `comment` is pure user input again; timelines render "from A to B" from
+    // these fields.
+    reassign_from: Field.lookup('sys_user', {
+      label: 'Reassigned From',
+      required: false,
+      group: 'Action',
+      description: 'User whose pending-approver slot was handed over (reassign actions only)',
+    }),
+
+    reassign_to: Field.lookup('sys_user', {
+      label: 'Reassigned To',
+      required: false,
+      group: 'Action',
+      description: 'User who received the pending-approver slot (reassign actions only)',
+    }),
+
     attachments: Field.file({
       label: 'Attachments',
       required: false,

packages/plugins/plugin-approvals/src/translations/en.objects.generated.ts

@@ -235,6 +235,14 @@ export const enObjects: NonNullable<TranslationData['objects']> = {
       comment: {
         label: "Comment"
       },
+      reassign_from: {
+        label: "Reassigned From",
+        help: "User whose pending-approver slot was handed over (reassign actions only)"
+      },
+      reassign_to: {
+        label: "Reassigned To",
+        help: "User who received the pending-approver slot (reassign actions only)"
+      },
       attachments: {
         label: "Attachments",
         help: "Files supporting this action — e.g. a signed contract or evidence (#3266)."

packages/plugins/plugin-approvals/src/translations/es-ES.objects.generated.ts

@@ -235,6 +235,14 @@ export const esESObjects: NonNullable<TranslationData['objects']> = {
       comment: {
         label: "Comentario"
       },
+      reassign_from: {
+        label: "Reasignado de",
+        help: "Usuario cuyo turno de aprobación pendiente fue traspasado (solo acciones de reasignación)"
+      },
+      reassign_to: {
+        label: "Reasignado a",
+        help: "Usuario que recibió el turno de aprobación pendiente (solo acciones de reasignación)"
+      },
       attachments: {
         label: "Adjuntos",
         help: "Archivos que respaldan esta acción, p. ej. un contrato firmado o pruebas (#3266)."

packages/plugins/plugin-approvals/src/translations/ja-JP.objects.generated.ts

@@ -235,6 +235,14 @@ export const jaJPObjects: NonNullable<TranslationData['objects']> = {
       comment: {
         label: "コメント"
       },
+      reassign_from: {
+        label: "引き継ぎ元",
+        help: "承認待ちスロットを引き渡したユーザー（引き継ぎ操作のみ）"
+      },
+      reassign_to: {
+        label: "引き継ぎ先",
+        help: "承認待ちスロットを受け取ったユーザー（引き継ぎ操作のみ）"
+      },
       attachments: {
         label: "添付ファイル",
         help: "この操作を裏付けるファイル——署名済み契約書や証憑など（#3266）。"

packages/plugins/plugin-approvals/src/translations/zh-CN.objects.generated.ts

@@ -235,6 +235,14 @@ export const zhCNObjects: NonNullable<TranslationData['objects']> = {
       comment: {
         label: "评论"
       },
+      reassign_from: {
+        label: "转出人",
+        help: "被移交待审批槽位的用户（仅转签操作）"
+      },
+      reassign_to: {
+        label: "转入人",
+        help: "接收待审批槽位的用户（仅转签操作）"
+      },
       attachments: {
         label: "附件",
         help: "支持该操作的文件——例如已签署的合同或证明材料（#3266）。"

packages/services/service-automation/src/builtin/crud-runas.test.ts

@@ -88,8 +88,11 @@ describe('flow.runAs identity enforcement at the data layer (#1888)', () => {
     for (const c of calls) {
       expect(c.ctx, `${c.op} got no context`).toBeTruthy();
       expect(c.ctx.isSystem, `${c.op} not elevated`).toBe(true);
-      // An elevated run is NOT attributed to the triggering user.
+      // An elevated run is NOT attributed to the triggering user…
       expect(c.ctx.userId).toBeUndefined();
+      // …but it IS attributed to the flow, so audit rows never read
+      // "Unknown user" (ADR-0014 D2, #4366).
+      expect(c.ctx.actor, `${c.op} lost the service-principal label`).toBe('svc:flow:sys');
     }
   });
 
@@ -181,9 +184,15 @@ describe('flow.runAs identity enforcement at the data layer (#1888)', () => {
 });
 
 describe('resolveRunDataContext (#1888 unit)', () => {
-  it("maps runAs:'system' to an elevated context", () => {
-    expect(resolveRunDataContext({ runAs: 'system', userId: 'u1' })).toEqual({
-      isSystem: true, positions: [], permissions: [],
+  it("maps runAs:'system' to an elevated context attributed to the flow (#4366)", () => {
+    expect(resolveRunDataContext({ runAs: 'system', userId: 'u1', flowName: 'mirror_status' })).toEqual({
+      isSystem: true, actor: 'svc:flow:mirror_status', positions: [], permissions: [],
+    });
+  });
+
+  it("labels a system run without a flow name as the generic automation principal (#4366)", () => {
+    expect(resolveRunDataContext({ runAs: 'system' })).toEqual({
+      isSystem: true, actor: 'svc:flow:automation', positions: [], permissions: [],
     });
   });
 
@@ -226,7 +235,7 @@ describe('resolveRunDataContext (#1888 unit)', () => {
   it("refuses rather than elevating — runAs:'system' stays the only route to isSystem", () => {
     expect(() => resolveRunDataContext({ runAs: 'user', flowRunId: 'r' })).toThrow();
     expect(resolveRunDataContext({ runAs: 'system', flowRunId: 'r' })).toEqual({
-      isSystem: true, positions: [], permissions: [], flowRunId: 'r',
+      isSystem: true, actor: 'svc:flow:automation', positions: [], permissions: [], flowRunId: 'r',
     });
   });
 });

packages/services/service-automation/src/engine.ts

@@ -1734,6 +1734,10 @@ export class AutomationEngine implements IAutomationService {
         const runContext: AutomationContext = {
             ...(context ?? {}),
             runAs: flow.runAs ?? 'user',
+            // `flowName` shares the same lifetime and construction point: it feeds
+            // audit attribution (`svc:flow:<name>` on ExecutionContext.actor) for
+            // runs that resolve no user (#4366).
+            flowName: flow.name,
             ...(runId ? { flowRunId: runId } : {}),
         };
 

packages/services/service-automation/src/runtime-identity.ts

@@ -14,6 +14,14 @@ import { markGuardRefusal } from './guard-refusal.js';
 export interface RunIdentityContext {
   /** Elevated, RLS-bypassing system principal (full access) when true. */
   isSystem: boolean;
+  /**
+   * Service-principal label for audit attribution (`ExecutionContext.actor`,
+   * ADR-0014 D2): `svc:flow:<flowName>` on a `runAs:'system'` run, which
+   * resolves no user. Without it the audit writer records `user_id=null,
+   * actor=null` and the history UI renders "Unknown user" (#4366).
+   * Attribution only — no security middleware keys on it.
+   */
+  actor?: string;
   /** Acting user id — drives owner/role RLS for `runAs:'user'` runs. */
   userId?: string;
   /** Acting user's role names (RLS parity with a direct REST request). */
@@ -105,8 +113,10 @@ export type RunDataContext = RunIdentityContext | RunProvenanceContext;
  * Translate a flow run's {@link AutomationContext} into the ObjectQL `context`
  * its CRUD nodes must pass, honoring `runAs` (ADR-0049 / #1888):
  *
- *  - `runAs:'system'` → `{ isSystem: true }` — the security middleware
- *    short-circuits, so the run reads/writes with full access, bypassing RLS.
+ *  - `runAs:'system'` → `{ isSystem: true, actor: 'svc:flow:<flowName>' }` —
+ *    the security middleware short-circuits, so the run reads/writes with full
+ *    access, bypassing RLS; the `actor` label keeps those writes attributable
+ *    in the audit log (ADR-0014 D2, #4366).
  *  - `runAs:'user'` (default) → the triggering user's identity
  *    (`{ userId, positions, permissions, tenantId? }`), so the security middleware
  *    enforces that user's row-level security. The run can never exceed the
@@ -139,7 +149,12 @@ export type RunDataContext = RunIdentityContext | RunProvenanceContext;
 export function resolveRunDataContext(context: AutomationContext | undefined): RunDataContext | undefined {
   const flowRunId = context?.flowRunId;
   if (context?.runAs === 'system') {
-    return { isSystem: true, positions: [], permissions: [], ...(flowRunId ? { flowRunId } : {}) };
+    // A system run resolves no user, so name the flow as the acting service
+    // principal (`svc:flow:<name>`, ADR-0014 D2) — the audit writer falls back
+    // to `session.actor` when `userId` is absent, which is what keeps these
+    // writes attributable instead of "Unknown user" (#4366).
+    const actor = `svc:flow:${context.flowName ?? 'automation'}`;
+    return { isSystem: true, actor, positions: [], permissions: [], ...(flowRunId ? { flowRunId } : {}) };
   }
   if (!context?.userId) {
     // #3760 — FAIL CLOSED. There is no identity to present, and presenting none