Skip to content

Commit 17a9dde

Browse files
committed
test(plugin-approvals): reproduce #15358 — a cascade-failed ancestor is reported as the repairable strand it is not
One fixture drives both shapes against a real AutomationEngine and a real ApprovalService: a child run stranded mid-continuation (repairable) and its parent, cascade-failed while parked at its subflow node (unrepairable). `inspectStrandedRequests` returns both as `runState: 'failed'`, while `restoreConsumedSuspension` re-arms one and refuses the other `NO_CONSUMED_SUSPENSION`. PIN 3 records the measurement that sends the ruled option B back for one more decision: `AutomationEngine.getRun` answers an `ExecutionLogEntry`, which carries neither `consumedSuspension` nor `consumedSuspensionDropped` for EITHER run — the engine keeps the snapshot off that interface on purpose, because `GET /automation/:name/runs/:runId` serves it verbatim. So widening the plugin-side declaration alone cannot separate the two rows. PIN 4 is the door control: the same strand carries `repairable: true` through `decide` and no envelope at all through `recall`. Part of #15358 Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01XpTx2tbq3pZRYAdoGt6E6Y
1 parent bf45e62 commit 17a9dde

1 file changed

Lines changed: 386 additions & 0 deletions

File tree

Lines changed: 386 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,386 @@
1+
// Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license.
2+
3+
/**
4+
* #15358 — THE REPRODUCTION: `inspectStrandedRequests` labels a repairable
5+
* strand and an UNREPAIRABLE cascade-failed ancestor identically.
6+
*
7+
* The card was filed from a reading of the sources. This file is the drive,
8+
* against a real `AutomationEngine` and a real `ApprovalService`, and it
9+
* reproduces: one fixture produces both rows, the inspection returns both as
10+
* `runState: 'failed'`, and the repair verb an operator would reach for next
11+
* answers `restored: true` for one and refuses the other.
12+
*
13+
* ## The two rows, from one fixture
14+
*
15+
* `deal_subflow` parks at an `approval`, and its approve edge leads to a
16+
* `subflow` node hosting `post_approval`, which parks at a SECOND `approval`
17+
* whose approve edge throws.
18+
*
19+
* - CHILD run — the #13909 strand: its resume consumed the pause and the
20+
* downstream node threw. The engine journals the consumed suspension, so
21+
* `restoreConsumedSuspension` re-arms it. REPAIRABLE.
22+
* - PARENT run — the cascade-failed ancestor (#15222's shape): it was parked
23+
* at its `subflow` node when the descendant failed, so `failAncestors` ran
24+
* `failSuspendedRun` on it, which consumes the pause and journals NOTHING.
25+
* `restoreConsumedSuspension` refuses it `NO_CONSUMED_SUSPENSION`. The
26+
* engine's own words for a terminal record carrying neither
27+
* `consumedSuspension` nor `consumedSuspensionDropped` (`RunRecord`):
28+
* "a run that reached a terminal state which was NOT a strand (completed,
29+
* cancelled, cascade-failed)". UNREPAIRABLE — and this row's decision did
30+
* advance its flow, which is the half the shared label denies.
31+
*
32+
* ## What must turn these assertions RED
33+
*
34+
* ⛔ The `runState` assertions below record what the inspection reports
35+
* TODAY. The maintainer ruling on this card (2026-09-04, decision batch #36,
36+
* option B) splits `StrandedRunState` into a repairable and an unrepairable
37+
* member and reports both, labelled apart — so whatever lands for B MUST turn
38+
* the "both come back identical" assertion red on purpose. It is written as a
39+
* single `toEqual` over both rows for exactly that reason: a split that
40+
* relabels only one of them still fails it.
41+
*
42+
* ## The measurement that sent the card back to the decision box
43+
*
44+
* `PIN 3` is the load-bearing one. B's first clause is "`getRun` widens to
45+
* carry the discriminator the engine already records". The engine records it
46+
* on the DURABLE `RunRecord`, and `AutomationEngine.getRun` answers an
47+
* `ExecutionLogEntry`, which carries neither field — deliberately: `recordLog`
48+
* says the snapshot is "a parameter rather than a field of
49+
* `ExecutionLogEntry`" because that interface "is served verbatim by
50+
* `GET /automation/:name/runs/:runId`". So widening the plugin-side
51+
* declaration alone cannot separate these two rows: on a real engine the
52+
* discriminator is absent for BOTH, and a classifier reading absence as "not a
53+
* strand" would answer UNREPAIRABLE for the repairable row — the #15555
54+
* false-negative harm, one surface over. Where the discriminator gets
55+
* published is a producer-side contract decision, and it is open.
56+
*
57+
* ## The control that makes the readings trustworthy
58+
*
59+
* `PIN 4` drives the SAME strand through a different door in the same run.
60+
* `decide` throws `RESUME_FAILED` carrying `{ finalized, decision, runId,
61+
* repairable: true }` (#13807, batch #37); `recall` reports the same strand as
62+
* a plain `resumeError` string with no envelope at all. ⇒ the difference is
63+
* the DOOR, not the strand — so a reading taken at one door is not a fact
64+
* about strands until the other door is driven too.
65+
*/
66+
67+
import { describe, it, expect, beforeEach } from 'vitest';
68+
import { AutomationEngine, InMemorySuspendedRunStore, installBuiltinNodes } from '@objectstack/service-automation';
69+
// [#4550] The engine double's write verbs route through ObjectQL's OWN dispatch
70+
// predicates rather than a hand-mirrored copy — a double looser than the engine
71+
// it stands in for is how #4434 shipped a dead REST route with its suite green.
72+
import { assertEngineDeleteDispatch, assertEngineUpdateDispatch } from '@objectstack/objectql';
73+
import { strandedDecisionDetails } from '@objectstack/types';
74+
import { ApprovalService } from './approval-service.js';
75+
import { registerApprovalNode } from './approval-node.js';
76+
77+
const SYSTEM_CTX = { isSystem: true, positions: [], permissions: [] } as any;
78+
const noopLogger: any = {
79+
info() {}, warn() {}, error() {}, debug() {}, child() { return noopLogger; },
80+
};
81+
82+
/** The downstream failure, in the card's own shape. */
83+
const DOWNSTREAM_FAILURE = 'update_record(crm_leave_request) failed: Record 9SEmlyRfw8D9-J7Z not found';
84+
85+
/** In-memory ObjectQL stand-in for the approvals tables. */
86+
function makeFakeEngine() {
87+
const tables = new Map<string, any[]>();
88+
const rows = (o: string) => (tables.get(o) ?? (tables.set(o, []), tables.get(o)!));
89+
const matches = (row: any, where: any) => Object.entries(where ?? {}).every(([k, v]) => {
90+
if (k.startsWith('$')) throw new Error(`fake engine: unsupported filter operator ${k}`);
91+
if (v && typeof v === 'object' && '$in' in (v as any)) return (v as any).$in.includes(row[k]);
92+
if (v && typeof v === 'object' && '$ne' in (v as any)) return row[k] !== (v as any).$ne;
93+
return row[k] === v;
94+
});
95+
return {
96+
tables,
97+
async find(object: string, opts: any = {}) {
98+
const where = opts.where ?? opts.filter ?? {};
99+
const out = rows(object).filter(r => matches(r, where));
100+
// The caller's bound is honoured by PRESENCE, never truthiness: `limit: 0`
101+
// is a real bound that must return no rows.
102+
const start = opts.offset ?? 0;
103+
const page = typeof opts.limit === 'number' ? out.slice(start, start + opts.limit) : out.slice(start);
104+
return page.map(r => ({ ...r }));
105+
},
106+
async insert(object: string, data: any) { rows(object).push({ ...data }); return { ...data }; },
107+
async update(object: string, data: any, options?: any) {
108+
const dispatch = assertEngineUpdateDispatch(data, options);
109+
const table = rows(object);
110+
if (dispatch.kind === 'multi') {
111+
let n = 0;
112+
for (let i = 0; i < table.length; i++) {
113+
if (matches(table[i], options?.where)) { table[i] = { ...table[i], ...data }; n++; }
114+
}
115+
return { updated: n };
116+
}
117+
const i = table.findIndex(r => r.id === dispatch.id);
118+
if (i >= 0) table[i] = { ...table[i], ...data };
119+
return i >= 0 ? { ...table[i] } : null;
120+
},
121+
async delete(object: string, options?: any) {
122+
const dispatch = assertEngineDeleteDispatch(options);
123+
const table = rows(object);
124+
if (dispatch.kind === 'multi') {
125+
const survivors = table.filter(r => !matches(r, options?.where));
126+
const deleted = table.length - survivors.length;
127+
table.splice(0, table.length, ...survivors);
128+
return { deleted };
129+
}
130+
const i = table.findIndex(r => r.id === dispatch.id);
131+
if (i >= 0) table.splice(i, 1);
132+
return { id: dispatch.id };
133+
},
134+
registerHook() {}, unregisterHooksByPackage() { return 0; }, async fire() {},
135+
};
136+
}
137+
138+
/** PARENT: approval, then a subflow that hosts a second approval. */
139+
const DEAL_SUBFLOW = {
140+
name: 'deal_subflow',
141+
label: 'Deal Approval With Follow-up Subflow',
142+
type: 'autolaunched',
143+
nodes: [
144+
{ id: 'start', type: 'start', label: 'Start' },
145+
{
146+
id: 'approve_step', type: 'approval', label: 'Manager Approval',
147+
config: { approvers: [{ type: 'user', value: 'u1' }] },
148+
},
149+
{
150+
id: 'sub', type: 'subflow', label: 'Post-approval subflow',
151+
config: { flowName: 'post_approval', outputVariable: 'subOut' },
152+
},
153+
{ id: 'rejected', type: 'mark', label: 'Rejected' },
154+
{ id: 'end', type: 'end', label: 'End' },
155+
],
156+
edges: [
157+
{ id: 'e1', source: 'start', target: 'approve_step' },
158+
{ id: 'e2', source: 'approve_step', target: 'sub', label: 'approve' },
159+
{ id: 'e3', source: 'approve_step', target: 'rejected', label: 'reject' },
160+
{ id: 'e4', source: 'sub', target: 'end' },
161+
{ id: 'e5', source: 'rejected', target: 'end' },
162+
],
163+
};
164+
165+
/** CHILD: a second approval whose approve edge is the node that throws. */
166+
const POST_APPROVAL = {
167+
name: 'post_approval',
168+
label: 'Post-approval follow-up',
169+
type: 'autolaunched',
170+
nodes: [
171+
{ id: 'cstart', type: 'start', label: 'Start' },
172+
{
173+
id: 'hold', type: 'approval', label: 'Second Approval',
174+
config: { approvers: [{ type: 'user', value: 'u2' }] },
175+
},
176+
{ id: 'boom', type: 'mark', label: 'Follow-up write' },
177+
{ id: 'child_rejected', type: 'mark', label: 'Child rejected' },
178+
{ id: 'cend', type: 'end', label: 'End' },
179+
],
180+
edges: [
181+
{ id: 'c1', source: 'cstart', target: 'hold' },
182+
{ id: 'c2', source: 'hold', target: 'boom', label: 'approve' },
183+
{ id: 'c3', source: 'hold', target: 'child_rejected', label: 'reject' },
184+
{ id: 'c4', source: 'boom', target: 'cend' },
185+
{ id: 'c5', source: 'child_rejected', target: 'cend' },
186+
],
187+
};
188+
189+
/** A single-approval flow, for the door control. */
190+
const DEAL_APPROVAL = {
191+
name: 'deal_approval',
192+
label: 'Deal Approval',
193+
type: 'autolaunched',
194+
nodes: [
195+
{ id: 'start', type: 'start', label: 'Start' },
196+
{
197+
id: 'approve_step', type: 'approval', label: 'Manager Approval',
198+
config: { approvers: [{ type: 'user', value: 'u1' }] },
199+
},
200+
{ id: 'on_approved', type: 'mark', label: 'Approved' },
201+
{ id: 'mark_rejected', type: 'mark', label: 'Rejected' },
202+
{ id: 'end', type: 'end', label: 'End' },
203+
],
204+
edges: [
205+
{ id: 'e1', source: 'start', target: 'approve_step' },
206+
{ id: 'e2', source: 'approve_step', target: 'on_approved', label: 'approve' },
207+
{ id: 'e3', source: 'approve_step', target: 'mark_rejected', label: 'reject' },
208+
{ id: 'e4', source: 'on_approved', target: 'end' },
209+
{ id: 'e5', source: 'mark_rejected', target: 'end' },
210+
],
211+
};
212+
213+
describe('#15358 — a cascade-failed ancestor is reported as the repairable strand it is not', () => {
214+
let data: ReturnType<typeof makeFakeEngine>;
215+
let service: ApprovalService;
216+
/** Node id → the message it throws when reached. */
217+
let throwOn: Record<string, string | undefined>;
218+
219+
beforeEach(() => {
220+
throwOn = {};
221+
data = makeFakeEngine();
222+
service = new ApprovalService({ engine: data as any, logger: noopLogger });
223+
});
224+
225+
/** One live process: real engine, real builtin nodes, real approval node, real service. */
226+
function boot() {
227+
const automation = new AutomationEngine(noopLogger, new InMemorySuspendedRunStore());
228+
installBuiltinNodes(automation, { logger: noopLogger, getService() { throw new Error('none'); } } as any);
229+
registerApprovalNode(automation, service, noopLogger);
230+
automation.registerNodeExecutor({
231+
type: 'mark',
232+
async execute(node: any) {
233+
const boom = throwOn[node.id];
234+
if (boom) throw new Error(boom);
235+
return { success: true };
236+
},
237+
} as never);
238+
automation.registerFlow('deal_subflow', DEAL_SUBFLOW as never);
239+
automation.registerFlow('post_approval', POST_APPROVAL as never);
240+
automation.registerFlow('deal_approval', DEAL_APPROVAL as never);
241+
service.attachAutomation(automation);
242+
return automation;
243+
}
244+
245+
const pendingRequest = async () =>
246+
(await data.find('sys_approval_request', { where: { status: 'pending' } }))[0];
247+
248+
/**
249+
* Drive the composition to its two terminal requests: a CHILD run stranded
250+
* mid-continuation, and a PARENT run cascade-failed while parked at its
251+
* `subflow` node.
252+
*/
253+
async function driveBothShapes(automation: AutomationEngine) {
254+
throwOn.boom = DOWNSTREAM_FAILURE;
255+
await automation.execute('deal_subflow', {
256+
object: 'crm_deal', record: { id: 'd1', amount: 100 }, userId: 'submitter',
257+
} as never);
258+
259+
const parentReq = await pendingRequest();
260+
const parentRunId = parentReq.flow_run_id as string;
261+
expect(parentRunId, 'the first request names the parent run').toBeTruthy();
262+
263+
// The parent's decision SUCCEEDS — it advances the flow into the subflow,
264+
// which is the whole reason its later `failed` row must not read as a
265+
// decision that never moved.
266+
const advanced = await service.decide(
267+
parentReq.id, { decision: 'approve', actorId: 'u1' }, SYSTEM_CTX,
268+
);
269+
expect(advanced.resumed, 'the parent decision DID advance the flow').toBe(true);
270+
expect(advanced.resumeError).toBeUndefined();
271+
expect(await automation.hasSuspendedRun(parentRunId), 'the parent is now parked at its subflow node').toBe(true);
272+
273+
const childReq = await pendingRequest();
274+
const childRunId = childReq.flow_run_id as string;
275+
expect(childRunId, 'the child run hosts the second approval').toBeTruthy();
276+
expect(childRunId).not.toBe(parentRunId);
277+
278+
// The child's decision strands the child AND cascade-fails the parent.
279+
const err = await service
280+
.decide(childReq.id, { decision: 'approve', actorId: 'u2' }, SYSTEM_CTX)
281+
.then(() => null, (e: Error) => e);
282+
expect(err?.message, 'the child door throws its stranded refusal').toMatch(/^RESUME_FAILED/);
283+
284+
return { parentReq, parentRunId, childReq, childRunId, childError: err };
285+
}
286+
287+
it('PIN 1 — the two rows come back with the SAME `runState`, and one of them cannot be repaired', async () => {
288+
const automation = boot();
289+
const { parentReq, parentRunId, childReq, childRunId } = await driveBothShapes(automation);
290+
291+
// ⚠️ INSPECT BEFORE RESTORING ANYTHING: `restoreConsumedSuspension` re-arms
292+
// the pause, which would move the row out of the inspection's answer.
293+
const out = await service.inspectStrandedRequests();
294+
expect(out.scanned).toBe(2);
295+
expect(out.undetermined).toBe(0);
296+
297+
const labelled = out.stranded
298+
.map(s => [s.requestId === parentReq.id ? 'cascade-failed ancestor' : 'genuine strand', s.runState])
299+
.sort((a, b) => a[0].localeCompare(b[0]));
300+
// ⛔ THE DEFECT, as one assertion: the label does not distinguish them.
301+
// Option B splits `StrandedRunState`, so this MUST go red when B lands.
302+
expect(labelled).toEqual([
303+
['cascade-failed ancestor', 'failed'],
304+
['genuine strand', 'failed'],
305+
]);
306+
307+
// …and both are reported with their decision durable, which is the true
308+
// half the report gets right for both rows.
309+
expect(out.stranded.map(s => s.status)).toEqual(['approved', 'approved']);
310+
expect(new Set(out.stranded.map(s => s.runId))).toEqual(new Set([parentRunId, childRunId]));
311+
void childReq;
312+
});
313+
314+
it('PIN 2 — the repair verb DOES tell them apart: one is re-armed, the other refused', async () => {
315+
// This is the fact the shared label hides, and the operator-visible cost
316+
// named on the card: a reported row the repair verb declines.
317+
const automation = boot();
318+
const { parentRunId, childRunId } = await driveBothShapes(automation);
319+
320+
const child = await automation.restoreConsumedSuspension(childRunId);
321+
expect(child.restored, 'the genuine strand is repairable').toBe(true);
322+
323+
const parent = await automation.restoreConsumedSuspension(parentRunId);
324+
expect(parent.restored, 'the cascade-failed ancestor is NOT').toBe(false);
325+
// The refusal is named, and its name is the engine's own reading of a
326+
// terminal record carrying no consumed-suspension snapshot.
327+
expect((parent as { refusal?: string }).refusal).toBe('NO_CONSUMED_SUSPENSION');
328+
});
329+
330+
it('PIN 3 — `getRun` carries NEITHER discriminator field, for either run', async () => {
331+
// Why B's first clause cannot be executed on the plugin side alone. The
332+
// engine records `consumedSuspension` / `consumedSuspensionDropped` on the
333+
// durable `RunRecord`; `getRun` answers an `ExecutionLogEntry`, and
334+
// `recordLog` keeps the snapshot OFF that interface on purpose because
335+
// `GET /automation/:name/runs/:runId` serves it verbatim.
336+
const automation = boot();
337+
const { parentRunId, childRunId } = await driveBothShapes(automation);
338+
339+
for (const [what, runId] of [['strand', childRunId], ['cascade', parentRunId]] as const) {
340+
const run = await automation.getRun(runId);
341+
// Positive control FIRST: this read reached a real terminal record, so
342+
// the absences below are absences IN one, not the shape of a null.
343+
expect(run, `${what}: the run history answers`).toBeTruthy();
344+
expect(run?.status, `${what}: and answers 'failed'`).toBe('failed');
345+
expect(typeof run?.error, `${what}: carrying the failure text`).toBe('string');
346+
// ⛔ The measurement: the property the inspector would have to read is
347+
// not on the object it reads.
348+
expect(Object.keys(run as object)).not.toContain('consumedSuspension');
349+
expect(Object.keys(run as object)).not.toContain('consumedSuspensionDropped');
350+
}
351+
});
352+
353+
it('PIN 4 — CONTROL: the same strand says `repairable: true` at one door and nothing at the other', async () => {
354+
// Without this, a reading taken at one door is not a reading about strands.
355+
throwOn.mark_rejected = DOWNSTREAM_FAILURE;
356+
const automation = boot();
357+
358+
// Door A — `decide`, which throws the #13807 envelope.
359+
await automation.execute('deal_approval', {
360+
object: 'crm_deal', record: { id: 'd2', amount: 10 }, userId: 'submitter',
361+
} as never);
362+
const decided = await pendingRequest();
363+
const decidedRunId = decided.flow_run_id as string;
364+
const err = await service
365+
.decide(decided.id, { decision: 'reject', actorId: 'u1' }, SYSTEM_CTX)
366+
.then(() => null, (e: Error) => e);
367+
expect(strandedDecisionDetails(err)).toEqual({
368+
finalized: true, decision: 'reject', runId: decidedRunId, repairable: true,
369+
});
370+
371+
// Door B — `recall`, same flow, same throwing node, same strand.
372+
await automation.execute('deal_approval', {
373+
object: 'crm_deal', record: { id: 'd3', amount: 20 }, userId: 'submitter',
374+
} as never);
375+
const recalled = await pendingRequest();
376+
const recalledRunId = recalled.flow_run_id as string;
377+
const result = await service.recall(recalled.id, { actorId: 'submitter' }, SYSTEM_CTX);
378+
379+
expect(result.resumed, 'the recall could not advance the run either').toBe(false);
380+
expect(result.resumeError, 'and reports it as a bare string').toContain(DOWNSTREAM_FAILURE);
381+
// ⛔ No envelope at this door — `repairable` is not stated at all…
382+
expect(strandedDecisionDetails(result as unknown)).toBeUndefined();
383+
// …while the strand behind it is every bit as repairable as door A's.
384+
expect((await automation.restoreConsumedSuspension(recalledRunId)).restored).toBe(true);
385+
});
386+
});

0 commit comments

Comments
 (0)