Skip to content

Commit 6508785

Browse files
committed
fix(plugin-approvals): every approval step records the run's trigger user as its submitter
Since a decision resumes its flow as the decider, the approval node's context.userId on a later step is the previous step's decider. The node now opens each step with the run's trigger user, the engine's own triggerUserId reading, and openNodeRequest treats an explicit null submitter as no submitter instead of falling back to the decider. Claude-Session: https://claude.ai/code/session_011TdZ8GjixNZFLz1xJD9zzK Co-authored-by: Claude <noreply@anthropic.com>
1 parent e3ac5ac commit 6508785

3 files changed

Lines changed: 45 additions & 6 deletions

File tree

‎packages/plugins/plugin-approvals/src/approval-node.ts‎

Lines changed: 32 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -83,6 +83,34 @@ function nestVariables(variables: Map<string, unknown>): Record<string, unknown>
8383
return vars;
8484
}
8585

86+
/**
87+
* The user who TRIGGERED the run — every step's submitter, whoever resumed the
88+
* run onto it (`ApprovalEscalationSchema.notifySubmitter`: "Notify the
89+
* original submitter on escalation").
90+
*
91+
* After an approval decision the engine runs the next segment as the DECIDER
92+
* (`ResumeSignal.resumedBy`, ruling A on `FlowSchema.runAs`), so
93+
* `context.userId` on a later step is the previous step's decider, never the
94+
* submitter. The engine keeps who started the run on the same context: at the
95+
* first switch it records `triggerUserId` (`null` when the trigger resolved no
96+
* user) and every later switch keeps it, persisted through each pause. Absent,
97+
* no switch happened and `userId` is still the trigger user. This is the
98+
* engine's own reading, `runTriggerUserId` in `@objectstack/service-automation`'s
99+
* `runtime-identity.ts`, mirrored here because this plugin deliberately does not
100+
* depend on service-automation (as {@link nestVariables} mirrors its condition
101+
* scope). The two-step pins in `later-step-original-submitter.integration.test.ts`
102+
* run it against the real engine.
103+
*
104+
* ⛔ Not `triggerUserId ?? userId`: a recorded `null` means nobody triggered
105+
* the run, and falling through would name the decider as its submitter.
106+
*/
107+
function runTriggerUser(context: { userId?: string; triggerUserId?: unknown } | undefined): string | null {
108+
if (context && Object.prototype.hasOwnProperty.call(context, 'triggerUserId')) {
109+
return typeof context.triggerUserId === 'string' && context.triggerUserId !== '' ? context.triggerUserId : null;
110+
}
111+
return context?.userId ?? null;
112+
}
113+
86114
/**
87115
* Register the `approval` node executor on the automation engine, plus the
88116
* `approval_revise` window the ADR-0044 send-back parks on. Idempotent at the
@@ -162,7 +190,10 @@ export function registerApprovalNode(
162190
flowName,
163191
flowLabel,
164192
nodeLabel: typeof node.label === 'string' ? node.label : undefined,
165-
submitterId: context?.userId ?? null,
193+
// The run's trigger user, not `context.userId`: after a decision
194+
// that is the decider, who acts in this segment (the envelope below)
195+
// but did not submit the run.
196+
submitterId: runTriggerUser(context),
166197
record,
167198
organizationId: context?.organizationId ?? context?.tenantId ?? null,
168199
// #3447 P2: flow variables (nested, as CEL conditions see them) — the

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

Lines changed: 10 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -3450,6 +3450,13 @@ export class ApprovalService implements IApprovalService {
34503450
flowLabel?: string;
34513451
/** Authored node label, snapshotted for inbox display. */
34523452
nodeLabel?: string;
3453+
/**
3454+
* Who submitted the run this step belongs to — the approval node passes
3455+
* its trigger user. `null` is an answer: the trigger resolved no user, so
3456+
* the step has no submitter. Only an OMITTED value falls back to the
3457+
* acting context's user: on a later step that context is the previous
3458+
* step's decider, who did not submit the run.
3459+
*/
34533460
submitterId?: string | null;
34543461
record?: any;
34553462
organizationId?: string | null;
@@ -3730,12 +3737,13 @@ export class ApprovalService implements IApprovalService {
37303737
const n = Array.isArray(prior) ? prior.length : 0;
37313738
if (n > 0) configSnapshot.__round = n + 1;
37323739
} catch { /* round display is best-effort */ }
3740+
const submitterId = input.submitterId !== undefined ? input.submitterId : (context.userId ?? null);
37333741
const row: any = {
37343742
id,
37353743
process_name: processName,
37363744
object_name: input.object,
37373745
record_id: input.recordId,
3738-
submitter_id: input.submitterId ?? context.userId ?? null,
3746+
submitter_id: submitterId,
37393747
status: 'pending',
37403748
// [#23889] The display snapshots the approval lists show: the flow's and
37413749
// the node's authored labels as the flow engine handed them over, and
@@ -3762,7 +3770,7 @@ export class ApprovalService implements IApprovalService {
37623770
await this.engine.insert('sys_approval_action', await this.actionRow(row, {
37633771
id: uid('aact'), request_id: id, organization_id: requestOrg,
37643772
step_name: input.nodeId, step_index: 0, action: 'submit',
3765-
actor_id: input.submitterId ?? context.userId ?? null, comment: null, created_at: now,
3773+
actor_id: submitterId, comment: null, created_at: now,
37663774
}), { context: SYSTEM_CTX });
37673775

37683776
// [#22824] The request's display says why it is with the administrators:

‎packages/plugins/plugin-approvals/src/later-step-original-submitter.integration.test.ts‎

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -206,8 +206,8 @@ describe('every step of an approval run names the run\'s original submitter (#24
206206
let now = Date.parse('2026-10-20T09:00:00.000Z');
207207
const emits: Emitted[] = [];
208208

209-
/** Start `flow` on a fresh deal as `userId`; answer the step it paused on. */
210-
const submit = async (flow: string, key: string, amount = 100, userId: string | undefined = SUBMITTER): Promise<any> => {
209+
/** Start `flow` on a fresh deal as `userId` (`null`: no user); answer the step it paused on. */
210+
const submit = async (flow: string, key: string, amount = 100, userId: string | null = SUBMITTER): Promise<any> => {
211211
const record = { id: `D_${key}`, title: key, amount, organization_id: ORG };
212212
await engine.insert('crm_deal', record, { context: SYSTEM } as any);
213213
const run = await automation.execute(flow, {
@@ -316,7 +316,7 @@ describe('every step of an approval run names the run\'s original submitter (#24
316316
});
317317

318318
it('a run whose trigger resolved no user opens its later step with no submitter, never the decider', async () => {
319-
const step1 = await submit('deal_chain', 'userless', 5000, undefined);
319+
const step1 = await submit('deal_chain', 'userless', 5000, null);
320320
expect(step1.submitter_id ?? null).toBeNull();
321321
await approve(step1, 'u_mgr');
322322
const step2 = await pendingOn('D_userless');

0 commit comments

Comments
 (0)