Skip to content

Commit e12c2e1

Browse files
committed
docs(automation): state the stored-flow impact, and pin the shape Studio emits
Two gaps found while verifying #4439. The #4414 changeset described the mechanism but not who it moves. `objectui`'s FlowEdgeInspector has always written `isDefault: true` when an out-edge is bound to a decision's default/else branch — into a key with zero readers, so that edge ran unconditionally alongside whichever branch matched. Enforcement therefore changes STORED flows, and mostly Studio's own: they now take exactly one branch. That is the fix, but it lands on existing data, which a reader of the release notes needs to know before upgrading. The same inspector also copies each branch's expression and label onto the edge it wires, so Studio emits the very double declaration the authoring guide told hand-writers to avoid. It routes correctly — the designer keeps the two sides in sync by construction — so the guide now says that plainly instead of forbidding a shape our own tool produces: redundant, not wrong, and dangerous only when the two disagree, which is the whole of #4414. Pins that exact emitted shape as a regression test, both directions. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Q8as8yR67v41xEdomiTba9
1 parent 30a0d57 commit e12c2e1

3 files changed

Lines changed: 96 additions & 3 deletions

File tree

.changeset/decision-branch-routing-enforced.md

Lines changed: 15 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -74,6 +74,21 @@ sibling — the actual hole), `flow-default-edge-with-condition` and
7474
Both of the first two fire on the pre-fix `convert-lead.flow.ts` and are silent
7575
after it.
7676

77+
## Effect on flows that already exist
78+
79+
Enforcing `isDefault` changes how a **stored** flow behaves, and the flows it
80+
changes are mostly Studio's own. `objectui`'s flow edge inspector has always
81+
written `isDefault: true` when you bind an out-edge to a decision's default/else
82+
branch — into a key with zero readers, so that edge ran unconditionally, in
83+
parallel with whichever branch actually matched. Those flows now take exactly
84+
one branch. That is the fix, but it is a behaviour change on existing data
85+
rather than only on newly authored metadata, so it is worth knowing before
86+
upgrading: a flow that quietly ran two paths will now run one.
87+
88+
Nothing changes for an edge that never carried the marker — `isDefault` defaults
89+
to `false`, and an ordinary unconditional out-edge still fans out in parallel
90+
exactly as before.
91+
7792
## The example app
7893

7994
`crm_convert_lead_wizard`'s guard is now a plain exclusive gateway: the

content/docs/automation/flows.mdx

Lines changed: 10 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -783,9 +783,16 @@ sibling), `flow-default-edge-with-condition` and `flow-multiple-default-edges`.
783783

784784
<Callout type="warn">
785785
A decision node that declares **no** `conditions` reports no branch at all — it
786-
is a plain gateway and its out-edges do the routing. Do not declare both:
787-
`config.conditions` *and* per-edge `condition`s on the same node means the node
788-
picks a branch, and then that branch's edge re-decides.
786+
is a plain gateway and its out-edges do the routing.
787+
788+
Declaring **both**`config.conditions` *and* per-edge `condition`s — is
789+
redundant but not wrong: the node picks a branch, and then that branch's edge
790+
re-decides with the same predicate. The Studio flow designer emits exactly this
791+
(it copies each branch's expression and label onto the edge it wires), and it
792+
routes correctly because the two are kept in sync by construction. Hand-written
793+
metadata has no such guarantee, which is the whole of #4414: when the two
794+
disagree, the node's branch wins the narrowing and the edge's predicate decides
795+
what actually runs. If you are writing the flow by hand, pick one.
789796
</Callout>
790797

791798
### Fault edges — handling a failed node

packages/services/service-automation/src/builtin/decision-branch-routing.test.ts

Lines changed: 71 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -296,3 +296,74 @@ describe('decision branch predicate is validated at registration (#4439)', () =>
296296
expect(() => engine.registerFlow('guard', flowWith("lead.status == 'converted'"))).not.toThrow();
297297
});
298298
});
299+
300+
/**
301+
* The shape objectui's flow designer actually emits, pinned.
302+
*
303+
* `FlowEdgeInspector.applyBranch()` copies a decision branch onto the edge it
304+
* wires: a guarded branch becomes `{ condition, label }`, and the `true`/empty
305+
* branch becomes `{ isDefault: true, label }`. So Studio has been writing
306+
* `isDefault` since long before anything read it (#4414) — every Studio
307+
* "default/else" edge ran unconditionally, in parallel with whichever branch
308+
* matched. These flows are the ones enforcement changes, and they must now take
309+
* exactly one path.
310+
*
311+
* It is also the double declaration the authoring guide tells hand-writers to
312+
* avoid — node `conditions[]` AND per-edge `condition`s. It is correct here
313+
* only because the designer keeps the two in sync by construction, which is
314+
* exactly why it is worth pinning rather than assuming.
315+
*/
316+
describe('objectui-authored decision shape (FlowEdgeInspector.applyBranch)', () => {
317+
let engine: AutomationEngine;
318+
let visited: string[];
319+
320+
beforeEach(() => {
321+
warnings.length = 0;
322+
visited = [];
323+
engine = new AutomationEngine(createTestLogger());
324+
registerLogicNodes(engine, createCtx());
325+
engine.registerNodeExecutor({
326+
type: 'mark',
327+
async execute(node) { visited.push(node.id); return { success: true }; },
328+
});
329+
engine.registerFlow('studio', {
330+
name: 'studio',
331+
label: 'Studio-authored',
332+
type: 'autolaunched',
333+
variables: [{ name: 'order_amount', type: 'number', isInput: true }],
334+
nodes: [
335+
{ id: 'start', type: 'start', label: 'Start' },
336+
{
337+
id: 'check', type: 'decision', label: 'Check Amount',
338+
config: {
339+
conditions: [
340+
{ label: 'High Value', expression: 'order_amount > 10000' },
341+
{ label: 'Standard', expression: 'true' },
342+
],
343+
},
344+
},
345+
{ id: 'escalate', type: 'mark', label: 'Escalate' },
346+
{ id: 'auto', type: 'mark', label: 'Auto approve' },
347+
],
348+
edges: [
349+
{ id: 'e1', source: 'start', target: 'check' },
350+
// The guarded branch: expression + label copied onto the edge.
351+
{ id: 'e2', source: 'check', target: 'escalate', label: 'High Value', condition: 'order_amount > 10000', isDefault: false },
352+
// The `true` branch: written as the BPMN default edge, no condition.
353+
{ id: 'e3', source: 'check', target: 'auto', label: 'Standard', isDefault: true },
354+
],
355+
});
356+
});
357+
358+
it('takes only the guarded branch when it matches', async () => {
359+
await engine.execute('studio', { params: { order_amount: 20000 } } as any);
360+
expect(visited).toEqual(['escalate']);
361+
expect(warnings).toHaveLength(0);
362+
});
363+
364+
it('takes only the default branch when it does not', async () => {
365+
await engine.execute('studio', { params: { order_amount: 5000 } } as any);
366+
expect(visited).toEqual(['auto']);
367+
expect(warnings).toHaveLength(0);
368+
});
369+
});

0 commit comments

Comments
 (0)