Skip to content

Commit 91f4c78

Browse files
baozhoutaoclaude
andauthored
fix(automation,objectql,spec,approvals): flow system-write audit attribution (#4366) + structured reassign hand-off (#4365) (#4403)
* fix(automation,objectql,spec): attribute runAs:'system' flow writes as svc:flow:<name> in the audit log (#4366) A runAs:'system' flow's data writes carried no attribution: 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 "Unknown user". The svc:* channel (ADR-0014 D2, ExecutionContext.actor) existed for exactly this writer class but was never wired end-to-end: - service-automation: resolveRunContext stamps flowName alongside runAs/flowRunId; resolveRunDataContext labels a system run's data context actor: 'svc:flow:<flowName>' (fallback svc:flow:automation). - objectql: buildSession propagates ExecutionContext.actor onto the hook session — without this hop the audit writer's `userId ?? session.actor` fallback was unreachable from the engine path. - spec: AutomationContext.flowName (engine-stamped provenance) and the hook session's optional `actor` field document the contract. userId still wins wherever present; the label is attribution only — no security middleware keys on it. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * feat(approvals,spec): structured reassign hand-off parties on sys_approval_action (#4365) A reassign's audit row encoded "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. - 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 actor_name, so timelines render "from A to B" without extra lookups. - ApprovalActionRow (spec contract) declares the four new fields; i18n bundles regenerated with zh-CN/ja-JP/es-ES translations. Pre-existing rows keep their legacy comment; clients should prefer the structured fields and fall back to comment otherwise. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> --------- Co-authored-by: Claude Fable 5 <noreply@anthropic.com>
1 parent 27f9072 commit 91f4c78

18 files changed

Lines changed: 262 additions & 14 deletions
Lines changed: 24 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,24 @@
1+
---
2+
"@objectstack/spec": minor
3+
"@objectstack/plugin-approvals": minor
4+
---
5+
6+
feat(approvals,spec): structured reassign hand-off parties on `sys_approval_action` (#4365)
7+
8+
A reassign's audit row used to encode "who handed the slot to whom" only inside
9+
a default free-text comment — `"<from_id> → <to_id>"`, two raw user ids — which
10+
clients could neither parse reliably nor render readably, so the approvals
11+
timeline showed opaque identifier soup for the single most important fact of
12+
the entry.
13+
14+
- `sys_approval_action` gains `reassign_from` / `reassign_to`
15+
(`lookup('sys_user')`), written by `ApprovalService.reassign()`.
16+
- `comment` is pure user input again: nothing is invented when the actor
17+
supplies none.
18+
- `listActions()` resolves both parties' display names into
19+
`reassign_from_name` / `reassign_to_name`, alongside the existing
20+
`actor_name`, so timelines can render "from A to B" without extra lookups.
21+
- `ApprovalActionRow` (spec contract) declares the four new fields.
22+
23+
Pre-existing rows keep their legacy comment; clients should prefer the
24+
structured fields when present and fall back to `comment` otherwise.
Lines changed: 29 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,29 @@
1+
---
2+
"@objectstack/spec": patch
3+
"@objectstack/service-automation": patch
4+
"@objectstack/objectql": patch
5+
---
6+
7+
fix(automation,objectql,spec): attribute `runAs:'system'` flow writes to the flow in the audit log (#4366)
8+
9+
A `runAs:'system'` flow's data writes carried no attribution at all: the run
10+
context resolved to `{ isSystem: true }` with no `userId` and no service
11+
principal, so the audit writer recorded `user_id=null, actor=null` and the
12+
record-history UI rendered every such row as "Unknown user" — business users
13+
read the flow's own status mirror as data corruption.
14+
15+
The `svc:*` attribution channel (ADR-0014 D2, `ExecutionContext.actor`) already
16+
existed for exactly this class of writer; it was simply never wired end-to-end:
17+
18+
- **service-automation**`resolveRunContext` now stamps `flowName` alongside
19+
`runAs`/`flowRunId`, and `resolveRunDataContext` labels a `runAs:'system'`
20+
run's data context `actor: 'svc:flow:<flowName>'` (fallback
21+
`svc:flow:automation`). Attribution only — no security middleware keys on it.
22+
- **objectql**`buildSession` propagates `ExecutionContext.actor` onto the
23+
hook session, closing the gap that left the audit writer's
24+
`userId ?? session.actor` fallback unreachable from the engine path.
25+
- **spec**`AutomationContext.flowName` (engine-stamped, provenance) and the
26+
hook session's optional `actor` field document the contract.
27+
28+
No behavior change for user-attributed writes: `userId` still wins wherever it
29+
is present.

content/docs/references/data/hook.mdx

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -37,7 +37,7 @@ const result = HookContext.parse(data);
3737
| **input** | `Record<string, any>` || Mutable input parameters |
3838
| **result** | `any` | optional | Operation result (After hooks only) |
3939
| **previous** | `Record<string, any>` | optional | Record state before operation |
40-
| **session** | `{ userId?: string; organizationId?: string; roles?: string[]; accessToken?: string; … }` | optional | Current session context |
40+
| **session** | `{ userId?: string; actor?: string; organizationId?: string; roles?: string[]; … }` | optional | Current session context |
4141
| **provenance** | `{ flowRunId?: string }` | optional | Server-stamped write provenance (never client-supplied, never an authorization input) |
4242
| **transaction** | `any` | optional | Database transaction handle |
4343
| **ql** | `any` || ObjectQL Engine Reference |

packages/objectql/src/engine.test.ts

Lines changed: 33 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -484,6 +484,39 @@ describe('ObjectQL Engine', () => {
484484
});
485485
});
486486

487+
/**
488+
* ADR-0014 D2 / #4366 — the service-principal label. A non-user write (a
489+
* `runAs:'system'` flow, a service token) carries `ExecutionContext.actor`;
490+
* the audit writer's `userId ?? session.actor` fallback is dead unless the
491+
* engine propagates it onto the hook session.
492+
*/
493+
describe('service-principal actor propagated to the hook session (#4366)', () => {
494+
beforeEach(async () => {
495+
engine.registerDriver(mockDriver, true);
496+
await engine.init();
497+
vi.mocked(SchemaRegistry.getObject).mockReturnValue({ name: 'task', fields: {} } as any);
498+
});
499+
500+
it('surfaces context.actor as session.actor for a system write', async () => {
501+
let session: any;
502+
engine.registerHook('afterInsert', async (ctx: any) => { session = ctx.session; }, { object: 'task' });
503+
504+
await engine.insert('task', { title: 'x' }, { context: { isSystem: true, actor: 'svc:flow:mirror_status' } as any });
505+
506+
expect(session).toMatchObject({ isSystem: true, actor: 'svc:flow:mirror_status' });
507+
});
508+
509+
it('omits session.actor when the context carries none (no anonymous label invented)', async () => {
510+
let session: any;
511+
engine.registerHook('afterInsert', async (ctx: any) => { session = ctx.session; }, { object: 'task' });
512+
513+
await engine.insert('task', { title: 'y' }, { context: { userId: 'u1' } as any });
514+
515+
expect(session.userId).toBe('u1');
516+
expect(session.actor).toBeUndefined();
517+
});
518+
});
519+
487520
describe('organizationId exposed to hooks as the blessed org name (#3280)', () => {
488521
beforeEach(async () => {
489522
engine.registerDriver(mockDriver, true);

packages/objectql/src/engine.ts

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

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

Lines changed: 28 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1121,12 +1121,38 @@ describe('ApprovalService (node era)', () => {
11211121

11221122
// ── thread interactions ─────────────────────────────────────────
11231123

1124-
it('reassign: hands the slot to a new approver and audits the move', async () => {
1124+
it('reassign: hands the slot to a new approver and audits the move on structured fields (#4365)', async () => {
11251125
const req = await svc.openNodeRequest(openInput(['u9', 'u2']), CTX);
11261126
const out = await svc.reassign(req.id, { actorId: 'u9', to: 'u7' }, asUser('u9'));
11271127
expect(out.request.pending_approvers).toEqual(['u7', 'u2']);
11281128
const actions = await svc.listActions(req.id, SYS);
1129-
expect(actions.at(-1)).toMatchObject({ action: 'reassign', actor_id: 'u9', comment: 'u9 → u7' });
1129+
const audit = actions.at(-1)!;
1130+
expect(audit).toMatchObject({ action: 'reassign', actor_id: 'u9', reassign_from: 'u9', reassign_to: 'u7' });
1131+
// No user comment → none invented. The old default baked raw user ids
1132+
// into user-facing text ("u9 → u7").
1133+
expect(audit.comment).toBeUndefined();
1134+
});
1135+
1136+
it('reassign: a user comment is stored verbatim alongside the structured fields (#4365)', async () => {
1137+
const req = await svc.openNodeRequest(openInput(['u9']), CTX);
1138+
await svc.reassign(req.id, { actorId: 'u9', to: 'u7', comment: 'On leave next week' }, asUser('u9'));
1139+
const actions = await svc.listActions(req.id, SYS);
1140+
expect(actions.at(-1)).toMatchObject({
1141+
action: 'reassign', reassign_from: 'u9', reassign_to: 'u7', comment: 'On leave next week',
1142+
});
1143+
});
1144+
1145+
it('reassign: listActions resolves the hand-off parties to display names (#4365)', async () => {
1146+
engine._tables['sys_user'] = [
1147+
{ id: 'u9', name: 'Grace Hopper', email: 'grace@example.com' },
1148+
{ id: 'u7', name: 'Ada Lovelace', email: 'ada@example.com' },
1149+
];
1150+
const req = await svc.openNodeRequest(openInput(['u9']), CTX);
1151+
await svc.reassign(req.id, { actorId: 'u9', to: 'u7' }, asUser('u9'));
1152+
const actions = await svc.listActions(req.id, SYS);
1153+
expect(actions.at(-1)).toMatchObject({
1154+
reassign_from_name: 'Grace Hopper', reassign_to_name: 'Ada Lovelace',
1155+
});
11301156
});
11311157

11321158
it('reassign: notifies the new approver via messaging', async () => {

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

Lines changed: 17 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -434,6 +434,9 @@ function rowFromAction(row: any): ApprovalActionRow {
434434
action: row.action,
435435
actor_id: row.actor_id ?? undefined,
436436
comment: row.comment ?? undefined,
437+
// Structured reassign hand-off parties (#4365).
438+
reassign_from: row.reassign_from ?? undefined,
439+
reassign_to: row.reassign_to ?? undefined,
437440
// Decision attachments (#3266): rich descriptors carrying the display name +
438441
// download URL, so consumers label/open them without reading `sys_file`.
439442
attachments: attachments.length ? attachments : undefined,
@@ -2276,7 +2279,11 @@ export class ApprovalService implements IApprovalService {
22762279
await this.engine.insert('sys_approval_action', {
22772280
id: uid('aact'), request_id: requestId, organization_id: raw.organization_id ?? null,
22782281
step_name: raw.flow_node_id ?? raw.current_step ?? null, step_index: 0, action: 'reassign',
2279-
actor_id: actorId, comment: input.comment ?? `${from}${to}`, created_at: now,
2282+
// The hand-off parties are STRUCTURED fields (#4365) — the old default
2283+
// comment (`"<from> → <to>"`) baked raw user ids into user-facing text.
2284+
// `comment` is pure user input: absent unless the actor wrote one.
2285+
actor_id: actorId, reassign_from: from, reassign_to: to,
2286+
comment: input.comment ?? null, created_at: now,
22802287
}, { context: SYSTEM_CTX });
22812288
// per_group / quorum (#3266): carry the delegated slot's group membership to
22822289
// the new approver in the snapshot, so their approval still counts for the
@@ -3620,13 +3627,20 @@ export class ApprovalService implements IApprovalService {
36203627
});
36213628
const actions = Array.isArray(rows) ? rows.map(rowFromAction) : [];
36223629
// Timeline display: resolve actor ids to names so the audit trail never
3623-
// shows a raw identifier. Role/team literals are already readable.
3630+
// shows a raw identifier. Role/team literals are already readable. The
3631+
// reassign hand-off parties (#4365) resolve through the same batch.
36243632
const names = await this.resolveUserNames(
3625-
actions.map(a => a.actor_id).filter(id => id && !id.includes(':')),
3633+
actions
3634+
.flatMap(a => [a.actor_id, a.reassign_from, a.reassign_to])
3635+
.filter(id => id && !id.includes(':')),
36263636
);
36273637
for (const a of actions as any[]) {
36283638
const n = a.actor_id ? names.get(String(a.actor_id)) : undefined;
36293639
if (n) a.actor_name = n;
3640+
const fromName = a.reassign_from ? names.get(String(a.reassign_from)) : undefined;
3641+
if (fromName) a.reassign_from_name = fromName;
3642+
const toName = a.reassign_to ? names.get(String(a.reassign_to)) : undefined;
3643+
if (toName) a.reassign_to_name = toName;
36303644
}
36313645
return actions;
36323646
}

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

Lines changed: 19 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -119,6 +119,25 @@ export const SysApprovalAction = ObjectSchema.create({
119119

120120
comment: Field.textarea({ label: 'Comment', required: false, group: 'Action' }),
121121

122+
// Structured hand-off parties for `action: 'reassign'` (#4365). Before
123+
// these existed the pair lived only inside a default free-text comment
124+
// ("<from_id> → <to_id>"), which no client could parse or render readably.
125+
// `comment` is pure user input again; timelines render "from A to B" from
126+
// these fields.
127+
reassign_from: Field.lookup('sys_user', {
128+
label: 'Reassigned From',
129+
required: false,
130+
group: 'Action',
131+
description: 'User whose pending-approver slot was handed over (reassign actions only)',
132+
}),
133+
134+
reassign_to: Field.lookup('sys_user', {
135+
label: 'Reassigned To',
136+
required: false,
137+
group: 'Action',
138+
description: 'User who received the pending-approver slot (reassign actions only)',
139+
}),
140+
122141
attachments: Field.file({
123142
label: 'Attachments',
124143
required: false,

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

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -235,6 +235,14 @@ export const enObjects: NonNullable<TranslationData['objects']> = {
235235
comment: {
236236
label: "Comment"
237237
},
238+
reassign_from: {
239+
label: "Reassigned From",
240+
help: "User whose pending-approver slot was handed over (reassign actions only)"
241+
},
242+
reassign_to: {
243+
label: "Reassigned To",
244+
help: "User who received the pending-approver slot (reassign actions only)"
245+
},
238246
attachments: {
239247
label: "Attachments",
240248
help: "Files supporting this action — e.g. a signed contract or evidence (#3266)."

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

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -235,6 +235,14 @@ export const esESObjects: NonNullable<TranslationData['objects']> = {
235235
comment: {
236236
label: "Comentario"
237237
},
238+
reassign_from: {
239+
label: "Reasignado de",
240+
help: "Usuario cuyo turno de aprobación pendiente fue traspasado (solo acciones de reasignación)"
241+
},
242+
reassign_to: {
243+
label: "Reasignado a",
244+
help: "Usuario que recibió el turno de aprobación pendiente (solo acciones de reasignación)"
245+
},
238246
attachments: {
239247
label: "Adjuntos",
240248
help: "Archivos que respaldan esta acción, p. ej. un contrato firmado o pruebas (#3266)."

0 commit comments

Comments
 (0)