Skip to content

Commit d5df816

Browse files
os-warrenclaude
andauthored
test(service-automation): pin the measured semantics of overlapping decision out-edge conditions (#16408)
A status-quo pin, not a contract. It records what the engine does today when two out-edges of one `decision` node carry conditions that both hold, so the pending semantic ruling has a baseline that cannot drift. Measured against the reported shape: - Take-every-match is real: both successors run in one execution. - "In parallel" is not: matching conditional edges are traversed one at a time, each successor fully executed before the next is evaluated. Only the unconditional bucket fans out through `Promise.all`, kept here as a positive control so the interleaving instrument is shown to detect concurrency where it exists. - A disjoint-condition control proves the counting method reads "exactly one" when the conditions do partition. - Nothing reports the multi-take: no warning, and no `skipped` step, so the run log is indistinguishable from a flow authored to run both branches. - Declared `config.conditions` is a distinct mode: first match wins, and the losing edge is narrowed away rather than gated (no step at all). No engine, spec or lint change: the card rules that changing evaluation semantics on a shipped node type is a behaviour change with its own ruling. Part of #15429 Claude-Session: https://claude.ai/code/session_01XpTx2tbq3pZRYAdoGt6E6Y Co-authored-by: Claude <noreply@anthropic.com>
1 parent fd75728 commit d5df816

1 file changed

Lines changed: 249 additions & 0 deletions

File tree

Lines changed: 249 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,249 @@
1+
// Copyright (c) 2025 ObjectStack. Licensed under the Apache-2.0 license.
2+
3+
import { describe, it, expect, beforeEach } from 'vitest';
4+
import { AutomationEngine } from '../engine.js';
5+
import { registerLogicNodes } from './logic-nodes.js';
6+
7+
/**
8+
* ⭐ A STATUS-QUO PIN, NOT A CONTRACT (#15429).
9+
*
10+
* Every assertion below records what the engine **does today** when two
11+
* out-edges of one `decision` node carry conditions that both hold. None of it
12+
* says the behaviour is right, and nothing here blesses it. The card that asked
13+
* for this file is explicit that changing evaluation semantics on a shipped node
14+
* type is a behaviour change with its own ruling, ⛔ not a bug fix — so the
15+
* first step is a baseline that cannot drift while that ruling is pending.
16+
*
17+
* ⇒ When the ruling lands, **this file is rewritten with it**. A failure here
18+
* after a deliberate semantic change is the pin doing its job, not a regression:
19+
* update the assertions and the prose together. A failure here after a change
20+
* that did NOT intend to touch branch selection is the drift it exists to catch.
21+
*
22+
* What the card reported, and what this file measured against it:
23+
*
24+
* • REPORTED (from one hotcrm deployment, reasoned backwards from a
25+
* reproduction): a decision with no declared `config.conditions` takes every
26+
* out-edge whose condition holds, **in parallel**.
27+
* • MEASURED here: the take-every-match half is real. The **in parallel** half
28+
* is not — matching conditional edges are traversed one at a time, each
29+
* successor fully executed before the next is evaluated (`traverseNext`
30+
* awaits inside the loop). Only the *unconditional* bucket fans out through
31+
* `Promise.all`, and this file keeps that fan-out as a positive control so
32+
* the interleaving instrument is shown to detect parallelism where it exists.
33+
*
34+
* The distinction matters for whoever writes the ruling: the hazard is
35+
* multi-branch execution, not concurrency. Two branches that both write the same
36+
* record run in a defined order, which is a different (and easier) starting
37+
* point than a genuine race.
38+
*
39+
* The two modes are separate on purpose (`logic-nodes.ts`, #4414): a decision
40+
* that DECLARES `config.conditions` reports the first matching entry's label and
41+
* traversal narrows to the edge carrying it; a decision that declares none
42+
* reports no branch at all and is a plain gateway whose out-edges route. The
43+
* last two tests pin both halves of that split, because "is undeclared a
44+
* distinct mode?" is the other question the ruling has to start from.
45+
*
46+
* On provenance, stated because the answer is "none": the take-every-match loop
47+
* predates the buckets it lives in. Before `cc8484224` (2026-02-21) traversal
48+
* was one loop that `continue`d past a closed gate and executed everything else;
49+
* that commit split conditional from unconditional edges, recorded a decision
50+
* about the unconditional half ("parallel branch execution (Promise.all for
51+
* unconditional edges)") and carried the conditional half over unchanged, under
52+
* a new comment reading `// Conditional edges: evaluate sequentially (mutually
53+
* exclusive)`. That comment is the only written trace of the assumption, and it
54+
* is an assumption: nothing in the engine, the schema or the linter makes
55+
* sibling conditions exclusive. ⛔ No commit message, ADR or doc records the
56+
* multi-take as a decision — searched with `git log -S` over `engine.ts` for the
57+
* loop's symbols, and `content/docs/automation/flows.mdx` describes the mode
58+
* ("BPMN exclusive gateway") without ever saying what happens when two
59+
* conditions hold at once.
60+
*/
61+
62+
/** One captured `warn` call, so "nothing reports this" is measured, not assumed. */
63+
const warnings: Array<{ msg: string; meta?: Record<string, unknown> }> = [];
64+
65+
function createTestLogger(): any {
66+
const logger: any = {
67+
info: () => {},
68+
warn: (msg: string, meta?: Record<string, unknown>) => { warnings.push({ msg: String(msg), meta }); },
69+
error: () => {},
70+
debug: () => {},
71+
child: () => logger,
72+
};
73+
return logger;
74+
}
75+
76+
function createCtx(): any {
77+
return { logger: createTestLogger(), getService: () => undefined };
78+
}
79+
80+
describe('decision with overlapping out-edge conditions — status-quo pin (#15429)', () => {
81+
let engine: AutomationEngine;
82+
/** `enter:<id>` / `exit:<id>` per visited successor — order AND nesting. */
83+
let trace: string[];
84+
85+
beforeEach(() => {
86+
warnings.length = 0;
87+
trace = [];
88+
engine = new AutomationEngine(createTestLogger());
89+
registerLogicNodes(engine, createCtx());
90+
// A terminal that yields between its two marks, so a fan-out that really
91+
// is concurrent interleaves (`enter,enter,exit,exit`) and a sequential
92+
// traversal nests (`enter,exit,enter,exit`). The yield is a macrotask,
93+
// not a duration: both orderings below are deterministic.
94+
engine.registerNodeExecutor({
95+
type: 'mark',
96+
async execute(node) {
97+
trace.push(`enter:${node.id}`);
98+
await new Promise(resolve => setTimeout(resolve, 0));
99+
trace.push(`exit:${node.id}`);
100+
return { success: true };
101+
},
102+
});
103+
// This harness is an embedded host, so it owes the ADR-0018 host half:
104+
// the vocabulary it contributes is complete (#4771). Without the seal the
105+
// first `execute()` warns about it and the zero-warning assertion below —
106+
// which is about branch reporting, not node types — would count that line.
107+
engine.sealNodeTypeVocabulary();
108+
});
109+
110+
/**
111+
* The hotcrm shape, reduced: one `decision`, two out-edges, no
112+
* `config.conditions` on the node. `a` and `b` are the two edge predicates;
113+
* an empty string means the edge carries no condition at all.
114+
*/
115+
function gatewayFlow(opts: { a: string; b: string; conditions?: Array<{ label: string; expression: string }> }) {
116+
return {
117+
name: 'gateway',
118+
label: 'Gateway',
119+
type: 'autolaunched' as const,
120+
variables: [{ name: 'lead', type: 'object', isInput: true }],
121+
nodes: [
122+
{ id: 'start', type: 'start' as const, label: 'Start' },
123+
{
124+
id: 'check', type: 'decision' as const, label: 'Check',
125+
...(opts.conditions ? { config: { conditions: opts.conditions } } : {}),
126+
},
127+
{ id: 'refuse', type: 'mark' as const, label: 'Refuse' },
128+
{ id: 'convert', type: 'mark' as const, label: 'Convert' },
129+
],
130+
edges: [
131+
{ id: 'e1', source: 'start', target: 'check' },
132+
{ id: 'e_refuse', source: 'check', target: 'refuse', label: 'Refuse', ...(opts.a ? { condition: opts.a } : {}) },
133+
{ id: 'e_convert', source: 'check', target: 'convert', label: 'Convert', ...(opts.b ? { condition: opts.b } : {}) },
134+
],
135+
};
136+
}
137+
138+
const run = (lead: Record<string, unknown>) => engine.execute('gateway', { params: { lead } } as any);
139+
140+
const stepsOfLastRun = async () => {
141+
const [log] = await engine.listRuns('gateway');
142+
return (log?.steps ?? []).map(s => ({ nodeId: s.nodeId, status: s.status, edgeId: s.skippedBy?.edgeId ?? null }));
143+
};
144+
145+
// ── The instrument, before any reading taken with it ──────────────────
146+
147+
it('CONTROL — disjoint edge conditions take exactly one successor', async () => {
148+
// Same node, same two edges, same record: only the predicates differ.
149+
// If this read "both" the counting method would prove nothing below.
150+
engine.registerFlow('gateway', gatewayFlow({
151+
a: "lead.status == 'suspected'",
152+
b: "lead.status == 'confirmed'",
153+
}));
154+
155+
await run({ status: 'confirmed' });
156+
157+
expect(trace).toEqual(['enter:convert', 'exit:convert']);
158+
// …and the branch not taken leaves a trace: a closed gate records a
159+
// `skipped` step naming the edge that closed it (#4354).
160+
expect(await stepsOfLastRun()).toContainEqual({ nodeId: 'refuse', status: 'skipped', edgeId: 'e_refuse' });
161+
});
162+
163+
// ── The reading ───────────────────────────────────────────────────────
164+
165+
it('takes EVERY out-edge whose condition holds — the reported hazard, confirmed', async () => {
166+
// The hotcrm pair verbatim in shape: a `Clean` guard spelled as a
167+
// negation, and a later `== confirmed` branch added beside it. For a
168+
// confirmed record both predicates are true, and the author's reading of
169+
// the node — "one of these" — is nowhere written down.
170+
engine.registerFlow('gateway', gatewayFlow({
171+
a: "lead.status != 'suspected'",
172+
b: "lead.status == 'confirmed'",
173+
}));
174+
175+
const result = await run({ status: 'confirmed' });
176+
177+
// Both successors run. The refusal screen renders AND the conversion
178+
// runs, in one execution — pinned as today's behaviour, ⛔ not endorsed.
179+
expect(trace.filter(t => t.startsWith('enter:'))).toEqual(['enter:refuse', 'enter:convert']);
180+
expect(result.success).toBe(true);
181+
});
182+
183+
it('takes them one at a time, NOT in parallel — the card says parallel; the engine does not', async () => {
184+
engine.registerFlow('gateway', gatewayFlow({
185+
a: "lead.status != 'suspected'",
186+
b: "lead.status == 'confirmed'",
187+
}));
188+
189+
await run({ status: 'confirmed' });
190+
191+
// Nested, not interleaved: `refuse` finishes before `convert` starts.
192+
expect(trace).toEqual(['enter:refuse', 'exit:refuse', 'enter:convert', 'exit:convert']);
193+
});
194+
195+
it('POSITIVE CONTROL — the same instrument reads interleaving on the unconditional fan-out', async () => {
196+
// Drop both conditions and the identical pair of edges lands in the
197+
// unconditional bucket, which really is `Promise.all`. This is what
198+
// proves the previous test measured sequencing rather than an instrument
199+
// that cannot see concurrency at all.
200+
engine.registerFlow('gateway', gatewayFlow({ a: '', b: '' }));
201+
202+
await run({ status: 'confirmed' });
203+
204+
expect(trace).toEqual(['enter:refuse', 'enter:convert', 'exit:refuse', 'exit:convert']);
205+
});
206+
207+
it('reports the multi-take NOWHERE — no warning, and no `skipped` step to notice it by', async () => {
208+
engine.registerFlow('gateway', gatewayFlow({
209+
a: "lead.status != 'suspected'",
210+
b: "lead.status == 'confirmed'",
211+
}));
212+
213+
await run({ status: 'confirmed' });
214+
215+
// Both edges opened, so neither records the `skipped` step that makes a
216+
// closed gate visible in the CONTROL above: the run log of a
217+
// two-branch execution is indistinguishable from a flow that was
218+
// *authored* to run both.
219+
const steps = await stepsOfLastRun();
220+
expect(steps.filter(s => s.status === 'skipped')).toEqual([]);
221+
expect(steps.filter(s => s.status === 'success').map(s => s.nodeId)).toEqual(['start', 'check', 'refuse', 'convert']);
222+
expect(warnings).toEqual([]);
223+
});
224+
225+
// ── The other half of the question: is undeclared a distinct mode? ────
226+
227+
it('declared `config.conditions` is a DIFFERENT mode — first match wins, even when two match', async () => {
228+
// The same two overlapping predicates, moved onto the node. The executor
229+
// returns on the first match (`logic-nodes.ts`), traversal narrows to the
230+
// edge carrying that label, and the second branch is never reached.
231+
engine.registerFlow('gateway', gatewayFlow({
232+
a: '', b: '',
233+
conditions: [
234+
{ label: 'Refuse', expression: "lead.status != 'suspected'" },
235+
{ label: 'Convert', expression: "lead.status == 'confirmed'" },
236+
],
237+
}));
238+
239+
await run({ status: 'confirmed' });
240+
241+
expect(trace).toEqual(['enter:refuse', 'exit:refuse']);
242+
// Narrowed away, not gated: the losing edge leaves no step at all — not
243+
// even the `skipped` one a closed gate writes. The two modes differ in
244+
// what they record as well as in what they run.
245+
const steps = await stepsOfLastRun();
246+
expect(steps.map(s => s.nodeId)).toEqual(['start', 'check', 'refuse']);
247+
expect(warnings).toEqual([]);
248+
});
249+
});

0 commit comments

Comments
 (0)