Skip to content

Commit e3ac5ac

Browse files
committed
test(plugin-approvals): pin every approval step's submitter as the run's trigger user
Reproduction first: on a real AutomationEngine + ApprovalService over SqlDriver, a later approval step opens with the previous step's decider as submitter_id. These pins go red on the unfixed approval node. Claude-Session: https://claude.ai/code/session_011TdZ8GjixNZFLz1xJD9zzK Co-authored-by: Claude <noreply@anthropic.com>
1 parent 41646a5 commit e3ac5ac

1 file changed

Lines changed: 384 additions & 0 deletions

File tree

Lines changed: 384 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,384 @@
1+
// Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license.
2+
3+
/**
4+
* Every step of an approval run names the run's ORIGINAL submitter (#24817).
5+
*
6+
* ## The defect
7+
*
8+
* Since an approval decision resumes its flow as the person who decided
9+
* (`ResumeSignal.resumedBy`, ruling A on `FlowSchema.runAs`), the segment after
10+
* a decision runs with `context.userId` = the DECIDER. The approval node opened
11+
* each step with `submitterId: context.userId`, so every step after the first
12+
* recorded the previous step's decider as its submitter: the "I Submitted"
13+
* list, recall / resubmit (submitter-only) and every submitter-facing notice
14+
* (`approval.returned`, `approval.request_info`, `approval.sla_breached`,
15+
* `approval.approved` / `approval.rejected`) moved to the wrong person. The
16+
* governing text is `ApprovalEscalationSchema.notifySubmitter`: "Notify the
17+
* original submitter on escalation".
18+
*
19+
* ## What this pins
20+
*
21+
* Driven through the real `ApprovalService` on a real `ObjectQL` over
22+
* `SqlDriver` (better-sqlite3, in-memory, real DDL), with the real
23+
* `AutomationEngine` running the flows, the real approval node opening each
24+
* step and the real resume-as-decider switch moving the segment's acting user:
25+
*
26+
* - each step of a three-step run carries the trigger user as `submitter_id`
27+
* (and as the actor of its `submit` timeline row), on both `runAs` postures,
28+
* and the "I Submitted" filter finds every step under that user. Who ACTS in
29+
* the resumed segment is not this file's question: the decider stays its
30+
* actor, pinned by `resume-decider-identity.integration.test.ts` in
31+
* `@objectstack/service-automation`;
32+
* - the step after a gateway-skipped step carries it too;
33+
* - a run whose trigger resolved no user opens its later step with no
34+
* submitter, never with the decider;
35+
* - recall and resubmit on step 2 admit the original submitter and refuse the
36+
* step-1 decider with the `FORBIDDEN` code;
37+
* - the decision outcome notice at the end of a two-step run reaches the
38+
* original submitter;
39+
* - control: a one-step run is unchanged.
40+
*
41+
* Every refusal carries its admitted control on the same request, so a build
42+
* that refuses everyone cannot pass one.
43+
*/
44+
45+
import { describe, it, expect, beforeAll, afterAll } from 'vitest';
46+
import { ObjectQL } from '@objectstack/objectql';
47+
import { SqlDriver } from '@objectstack/driver-sql';
48+
import { AutomationEngine, registerLogicNodes } from '@objectstack/service-automation';
49+
import { APPROVAL_REVISE_NODE_TYPE } from '@objectstack/spec/automation';
50+
import type { EngineQueryOptions } from '@objectstack/spec/data';
51+
import { ApprovalService } from './approval-service.js';
52+
import { ApprovalsServicePlugin } from './approvals-plugin.js';
53+
import { registerApprovalNode } from './approval-node.js';
54+
55+
const SYSTEM = { isSystem: true, positions: [], permissions: [] } as any;
56+
const asUser = (userId: string) => ({ userId, positions: [], permissions: [] }) as any;
57+
const silent = { info: () => {}, warn: () => {}, error: () => {}, debug: () => {} } as any;
58+
59+
const ORG = 'org_later_step';
60+
const SUBMITTER = 'u_sub';
61+
62+
const USERS = [
63+
{ id: SUBMITTER, name: 'Sam Submitter', email: 'sam@example.com' },
64+
{ id: 'u_mgr', name: 'Mona Manager', email: 'mona@example.com' },
65+
{ id: 'u_exec', name: 'Eve Exec', email: 'eve@example.com' },
66+
{ id: 'u_cfo', name: 'Carl CFO', email: 'carl@example.com' },
67+
];
68+
69+
const sysUser = {
70+
name: 'sys_user',
71+
label: 'User',
72+
fields: {
73+
id: { name: 'id', label: 'Id', type: 'text' as const, primaryKey: true },
74+
name: { name: 'name', label: 'Name', type: 'text' as const },
75+
email: { name: 'email', label: 'Email', type: 'text' as const },
76+
},
77+
};
78+
const deal = {
79+
name: 'crm_deal',
80+
label: 'Deal',
81+
fields: {
82+
id: { name: 'id', label: 'Id', type: 'text' as const, primaryKey: true },
83+
title: { name: 'title', label: 'Title', type: 'text' as const },
84+
amount: { name: 'amount', label: 'Amount', type: 'number' as const },
85+
},
86+
};
87+
88+
/** The plugin's own object definitions, as its manifest registers them. */
89+
async function packageObjects(): Promise<any[]> {
90+
let manifest: any;
91+
const ctx: any = {
92+
getService: (name: string) => {
93+
if (name === 'manifest') return { register: (m: any) => { manifest = m; } };
94+
throw new Error(`no service '${name}'`);
95+
},
96+
logger: silent,
97+
};
98+
await new ApprovalsServicePlugin().init(ctx);
99+
return manifest.objects;
100+
}
101+
102+
const user = (value: string) => ({ type: 'user', value });
103+
const step = (id: string, label: string, approver: string) =>
104+
({ id, type: 'approval', label, config: { approvers: [user(approver)] } });
105+
106+
/** One approval step between start and two ends: the control. */
107+
const ONE_STEP = {
108+
name: 'deal_one_step', label: 'Deal One Step', type: 'autolaunched',
109+
nodes: [
110+
{ id: 'start', type: 'start', label: 'Start' },
111+
step('mgr', 'Manager Review', 'u_mgr'),
112+
{ id: 'ok', type: 'end', label: 'Approved' },
113+
{ id: 'no', type: 'end', label: 'Rejected' },
114+
],
115+
edges: [
116+
{ id: 'e1', source: 'start', target: 'mgr' },
117+
{ id: 'e2', source: 'mgr', target: 'ok', label: 'approve' },
118+
{ id: 'e3', source: 'mgr', target: 'no', label: 'reject' },
119+
],
120+
};
121+
122+
/**
123+
* A manager, then — above the threshold — an executive, then a CFO; below it
124+
* the gateway skips the executive and the manager's approval lands on the CFO.
125+
* The showcase's `showcase_budget_approval` (`manager_review` → `exec_review`
126+
* above 500000) and `showcase_dynamic_approval` (`lead_review` → `co_sign`)
127+
* walk the same resumed-segment path.
128+
*/
129+
function chain(name: string, runAs?: 'user' | 'system'): any {
130+
return {
131+
name, label: name, type: 'autolaunched',
132+
...(runAs ? { runAs } : {}),
133+
nodes: [
134+
{ id: 'start', type: 'start', label: 'Start' },
135+
step('mgr', 'Manager Review', 'u_mgr'),
136+
{ id: 'big', type: 'decision', label: 'Above 1000?' },
137+
step('exec', 'Executive Review', 'u_exec'),
138+
step('cfo', 'CFO Review', 'u_cfo'),
139+
{ id: 'ok', type: 'end', label: 'Approved' },
140+
{ id: 'no', type: 'end', label: 'Rejected' },
141+
],
142+
edges: [
143+
{ id: 'e1', source: 'start', target: 'mgr' },
144+
{ id: 'e2', source: 'mgr', target: 'big', label: 'approve' },
145+
{ id: 'e3', source: 'mgr', target: 'no', label: 'reject' },
146+
{ id: 'e4', source: 'big', target: 'exec', label: 'true', condition: 'record.amount > 1000' },
147+
{ id: 'e5', source: 'big', target: 'cfo', label: 'false', condition: 'record.amount <= 1000' },
148+
{ id: 'e6', source: 'exec', target: 'cfo', label: 'approve' },
149+
{ id: 'e7', source: 'exec', target: 'no', label: 'reject' },
150+
{ id: 'e8', source: 'cfo', target: 'ok', label: 'approve' },
151+
{ id: 'e9', source: 'cfo', target: 'no', label: 'reject' },
152+
],
153+
};
154+
}
155+
156+
/** Two steps; the second can send back into its own ADR-0044 revise window. */
157+
const REVISABLE = {
158+
name: 'deal_revisable', label: 'Deal Revisable', type: 'autolaunched',
159+
nodes: [
160+
{ id: 'start', type: 'start', label: 'Start' },
161+
step('mgr', 'Manager Review', 'u_mgr'),
162+
step('exec', 'Executive Review', 'u_exec'),
163+
{ id: 'exec_rework', type: APPROVAL_REVISE_NODE_TYPE, label: 'Awaiting Revision' },
164+
{ id: 'ok', type: 'end', label: 'Approved' },
165+
{ id: 'no', type: 'end', label: 'Rejected' },
166+
],
167+
edges: [
168+
{ id: 'e1', source: 'start', target: 'mgr' },
169+
{ id: 'e2', source: 'mgr', target: 'exec', label: 'approve' },
170+
{ id: 'e3', source: 'mgr', target: 'no', label: 'reject' },
171+
{ id: 'e4', source: 'exec', target: 'ok', label: 'approve' },
172+
{ id: 'e5', source: 'exec', target: 'no', label: 'reject' },
173+
{ id: 'e6', source: 'exec', target: 'exec_rework', label: 'revise' },
174+
{ id: 'e7', source: 'exec_rework', target: 'exec', label: 'resubmit', type: 'back' },
175+
],
176+
};
177+
178+
/**
179+
* The service-seam form of the ADR-0112 refusal: the code prefix
180+
* `@objectstack/rest`'s `handleApprovalError` maps to the wire `code` (and
181+
* `FORBIDDEN` to status 403, pinned live in `rest-approvals-wire-codes.test.ts`)
182+
* before stripping it from the body. `undefined` when the call was admitted.
183+
*/
184+
async function refusalCode(p: Promise<unknown>): Promise<string | undefined> {
185+
try {
186+
await p;
187+
return undefined;
188+
} catch (err: any) {
189+
const code = /^([A-Z][A-Z0-9_]*):\s/.exec(String(err?.message ?? ''))?.[1];
190+
// A throw with no code prefix is not a refusal the door can classify.
191+
expect(code, `a coded refusal, got: ${String(err?.message ?? err)}`).toBeDefined();
192+
return code;
193+
}
194+
}
195+
196+
interface Emitted {
197+
topic: string;
198+
audience: string[];
199+
source?: { object: string; id: string };
200+
}
201+
202+
describe('every step of an approval run names the run\'s original submitter (#24817)', () => {
203+
let engine: ObjectQL;
204+
let automation: AutomationEngine;
205+
let svc: ApprovalService;
206+
let now = Date.parse('2026-10-20T09:00:00.000Z');
207+
const emits: Emitted[] = [];
208+
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> => {
211+
const record = { id: `D_${key}`, title: key, amount, organization_id: ORG };
212+
await engine.insert('crm_deal', record, { context: SYSTEM } as any);
213+
const run = await automation.execute(flow, {
214+
object: 'crm_deal', record, ...(userId ? { userId } : {}),
215+
} as any);
216+
expect(run.status, `${flow} on D_${key} paused on its first step`).toBe('paused');
217+
return pendingOn(`D_${key}`);
218+
};
219+
/** The one pending request on a deal, or `undefined`. */
220+
const pendingOn = async (recordId: string): Promise<any> => {
221+
const rows: any[] = await engine.find('sys_approval_request', {
222+
where: { record_id: recordId, status: 'pending' }, context: SYSTEM,
223+
} satisfies EngineQueryOptions);
224+
expect(rows.length, `at most one pending request on ${recordId}`).toBeLessThanOrEqual(1);
225+
return rows[0];
226+
};
227+
const stored = async (requestId: string): Promise<any> =>
228+
engine.findOne('sys_approval_request', { where: { id: requestId } }, { context: SYSTEM });
229+
/** Who the request's own `submit` timeline row names. */
230+
const submitActor = async (requestId: string): Promise<unknown> => {
231+
const rows: any[] = await engine.find('sys_approval_action', {
232+
where: { request_id: requestId, action: 'submit' }, context: SYSTEM,
233+
} satisfies EngineQueryOptions);
234+
expect(rows.length, `one submit row on ${requestId}`).toBe(1);
235+
return rows[0].actor_id ?? null;
236+
};
237+
/** `decider` approves `req`; the run resumes as them onto its next node. */
238+
const approve = async (req: any, decider: string) => {
239+
const out = await svc.decide(req.id, { decision: 'approve', actorId: decider }, asUser(decider));
240+
expect([out.finalized, out.resumed], `${decider} approved ${req.flow_node_id}`).toEqual([true, true]);
241+
};
242+
243+
beforeAll(async () => {
244+
engine = new ObjectQL();
245+
engine.registerDriver(new SqlDriver({
246+
client: 'better-sqlite3', connection: { filename: ':memory:' }, useNullAsDefault: true,
247+
}), true);
248+
await engine.init();
249+
for (const def of [sysUser, deal, ...(await packageObjects())]) {
250+
engine.registry.registerObject(def as any, 'approvals-test', 'approvals-test');
251+
}
252+
await engine.syncSchemas();
253+
for (const u of USERS) await engine.insert('sys_user', u, { context: SYSTEM } as any);
254+
255+
automation = new AutomationEngine(silent);
256+
// The gateway is a `decision` node, one of the built-ins the automation
257+
// plugin seeds; a bare engine starts without them.
258+
registerLogicNodes(automation, { logger: silent } as any);
259+
svc = new ApprovalService({
260+
engine: engine as any,
261+
clock: { now: () => new Date(now += 1000) },
262+
logger: silent,
263+
});
264+
svc.attachAutomation(automation as any);
265+
svc.attachMessaging({ async emit(input) { emits.push(input as Emitted); } });
266+
registerApprovalNode(automation as any, svc, silent);
267+
268+
automation.registerFlow('deal_one_step', ONE_STEP as any);
269+
automation.registerFlow('deal_chain', chain('deal_chain'));
270+
automation.registerFlow('deal_chain_system', chain('deal_chain_system', 'system'));
271+
automation.registerFlow('deal_revisable', REVISABLE as any);
272+
});
273+
274+
afterAll(async () => {
275+
try { await engine?.destroy(); } catch { /* noop */ }
276+
});
277+
278+
for (const flow of ['deal_chain', 'deal_chain_system'] as const) {
279+
it(`${flow}: each of three steps names the trigger user as submitter, whoever decided the step before`, async () => {
280+
const step1 = await submit(flow, `${flow}_three`, 5000);
281+
expect([step1.flow_node_id, step1.submitter_id]).toEqual(['mgr', SUBMITTER]);
282+
283+
await approve(step1, 'u_mgr');
284+
const step2 = await pendingOn(`D_${flow}_three`);
285+
expect([step2?.flow_node_id, step2?.flow_run_id]).toEqual(['exec', step1.flow_run_id]);
286+
expect(step2.submitter_id, 'step 2 names the trigger user, not the step-1 decider').toBe(SUBMITTER);
287+
expect(await submitActor(step2.id)).toBe(SUBMITTER);
288+
289+
// A second decision by a different person: the trigger the engine keeps
290+
// survives the second switch of the segment's acting user.
291+
await approve(step2, 'u_exec');
292+
const step3 = await pendingOn(`D_${flow}_three`);
293+
expect([step3?.flow_node_id, step3?.flow_run_id]).toEqual(['cfo', step1.flow_run_id]);
294+
expect(step3.submitter_id, 'step 3 names the trigger user, not the step-2 decider').toBe(SUBMITTER);
295+
expect(await submitActor(step3.id)).toBe(SUBMITTER);
296+
297+
// The "I Submitted" filter finds all three steps under the trigger user
298+
// and none under a decider.
299+
const listed = async (submitterId: string) =>
300+
(await svc.listRequests({ submitterId, recordId: `D_${flow}_three` }, SYSTEM))
301+
.map((r) => r.current_step).sort();
302+
expect(await listed(SUBMITTER)).toEqual(['cfo', 'exec', 'mgr']);
303+
expect(await listed('u_mgr')).toEqual([]);
304+
expect(await listed('u_exec')).toEqual([]);
305+
});
306+
}
307+
308+
it('the step after a gateway-skipped step names the trigger user as submitter', async () => {
309+
// The graph still holds the executive step; this deal's amount routes around it.
310+
const step1 = await submit('deal_chain', 'skip', 500);
311+
await approve(step1, 'u_mgr');
312+
const next = await pendingOn('D_skip');
313+
expect([next?.flow_node_id, next?.flow_run_id]).toEqual(['cfo', step1.flow_run_id]);
314+
expect(next.submitter_id, 'the step after the skipped one names the trigger user').toBe(SUBMITTER);
315+
expect(await submitActor(next.id)).toBe(SUBMITTER);
316+
});
317+
318+
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);
320+
expect(step1.submitter_id ?? null).toBeNull();
321+
await approve(step1, 'u_mgr');
322+
const step2 = await pendingOn('D_userless');
323+
expect(step2?.flow_node_id).toBe('exec');
324+
expect(step2.submitter_id ?? null, 'nobody submitted this run, so nobody submitted step 2').toBeNull();
325+
expect(await submitActor(step2.id)).toBeNull();
326+
});
327+
328+
it('recall on step 2: the step-1 decider is refused FORBIDDEN; the original submitter is admitted', async () => {
329+
const step1 = await submit('deal_chain', 'recall', 5000);
330+
await approve(step1, 'u_mgr');
331+
const step2 = await pendingOn('D_recall');
332+
expect(step2?.flow_node_id).toBe('exec');
333+
334+
expect(await refusalCode(svc.recall(step2.id, { actorId: 'u_mgr' }, asUser('u_mgr')))).toBe('FORBIDDEN');
335+
expect((await stored(step2.id)).status, 'the refused recall moved nothing').toBe('pending');
336+
337+
expect(await refusalCode(svc.recall(step2.id, { actorId: SUBMITTER }, asUser(SUBMITTER)))).toBeUndefined();
338+
expect((await stored(step2.id)).status).toBe('recalled');
339+
});
340+
341+
it('resubmit on step 2: the step-1 decider is refused FORBIDDEN; the original submitter is admitted', async () => {
342+
const step1 = await submit('deal_revisable', 'resubmit', 5000);
343+
await approve(step1, 'u_mgr');
344+
const step2 = await pendingOn('D_resubmit');
345+
expect([step2?.flow_node_id, step2?.submitter_id]).toEqual(['exec', SUBMITTER]);
346+
347+
await svc.sendBack(step2.id, { actorId: 'u_exec', comment: 'Attach the quote.' }, asUser('u_exec'));
348+
expect((await stored(step2.id)).status).toBe('returned');
349+
// The send-back tells the submitter it came back — the original one.
350+
expect(emits.filter((e) => e.topic === 'approval.returned' && e.source?.id === step2.id).map((e) => e.audience))
351+
.toEqual([[SUBMITTER]]);
352+
353+
expect(await refusalCode(svc.resubmit(step2.id, { actorId: 'u_mgr' }, asUser('u_mgr')))).toBe('FORBIDDEN');
354+
expect((await stored(step2.id)).status, 'the refused resubmit moved nothing').toBe('returned');
355+
expect(await pendingOn('D_resubmit')).toBeUndefined();
356+
357+
expect(await refusalCode(svc.resubmit(step2.id, { actorId: SUBMITTER }, asUser(SUBMITTER)))).toBeUndefined();
358+
const round2 = await pendingOn('D_resubmit');
359+
expect([round2?.flow_node_id, round2?.flow_run_id, round2?.submitter_id])
360+
.toEqual(['exec', step1.flow_run_id, SUBMITTER]);
361+
});
362+
363+
it('the decision that ends a two-step run tells the original submitter (approval.approved)', async () => {
364+
const step1 = await submit('deal_revisable', 'outcome', 5000);
365+
await approve(step1, 'u_mgr');
366+
const step2 = await pendingOn('D_outcome');
367+
await approve(step2, 'u_exec');
368+
expect(await pendingOn('D_outcome')).toBeUndefined();
369+
expect((await stored(step2.id)).status).toBe('approved');
370+
371+
const told = emits.filter((e) => ['approval.approved', 'approval.rejected'].includes(e.topic)
372+
&& [step1.id, step2.id].includes(e.source?.id ?? ''));
373+
expect(told.map((e) => [e.topic, e.source?.id, e.audience])).toEqual([['approval.approved', step2.id, [SUBMITTER]]]);
374+
});
375+
376+
it('control: a one-step run names its trigger user, and that user may recall it', async () => {
377+
const only = await submit('deal_one_step', 'control');
378+
expect([only.flow_node_id, only.submitter_id]).toEqual(['mgr', SUBMITTER]);
379+
expect(await submitActor(only.id)).toBe(SUBMITTER);
380+
expect(await refusalCode(svc.recall(only.id, { actorId: 'u_mgr' }, asUser('u_mgr')))).toBe('FORBIDDEN');
381+
expect(await refusalCode(svc.recall(only.id, { actorId: SUBMITTER }, asUser(SUBMITTER)))).toBeUndefined();
382+
expect((await stored(only.id)).status).toBe('recalled');
383+
});
384+
});

0 commit comments

Comments
 (0)