You signed in with another tab or window. Reload to refresh your session.You signed out in another tab or window. Reload to refresh your session.You switched accounts on another tab or window. Reload to refresh your session.Dismiss alert
fix(automation): enforce isDefault and stop swallowing an unclaimable branch label (#4440)
* fix(automation): enforce isDefault and stop swallowing an unclaimable branch label (#4414)
A `decision` node advertised three ways to split a path and only
`edge.condition` did anything. `FlowEdgeSchema.isDefault` had zero readers
outside its own declaration, and the `conditions[].label` → `branchLabel`
route matched 0 out-edge labels across every example app before falling back
to the full edge set in silence. Stacked, the two shipped a guard that does
not guard: `crm_convert_lead_wizard` showed an already-converted lead the
abort screen AND walked it into the conversion wizard behind it.
The three mechanisms now compose as one model in `traverseNext`:
`branchLabel` narrows the edge set, `condition` gates each edge, `isDefault`
catches whatever is left.
- `isDefault` is enforced as the BPMN default flow: traversed only when no
conditional sibling matched, and kept out of the unconditional parallel
fan-out. Passed over because a real branch won, its target records the
same `skipped` step a closed gate does (#4354).
- A branch label no out-edge carries is logged instead of swallowed.
Traversal still falls back to the full edge set — a run mid-flight must
not die on a metadata error — but says which branch was computed and
which labels exist.
- A decision that declares no `conditions` reports no branch. It used to
report `'default'` regardless, a label no out-edge in the repo carried,
which is why every decision node fell back to the full edge set. The
sentinel survives for the case it describes (declared conditions, none
matched) and is now claimed by the `isDefault` edge as well.
- `conditions[].expression` is evaluated as the bare CEL it is declared to
be. The raw string went to the legacy `{var}` template path, where a
dotted reference cannot resolve and the branch is decided by string
comparison; a brace-in-CEL predicate now fails loudly (ADR-0032 §1c).
Caught at authoring time too, since a wrong route is silent at run time by
nature (Prime Directive #12): `flow-branch-label-unmatched`,
`flow-decision-unconditional-branch`, `flow-default-edge-with-condition` and
`flow-multiple-default-edges`. The first two fire on the pre-fix
`convert-lead.flow.ts` and are silent after it.
app-crm's guard is now a plain exclusive gateway — the redundant
`config.conditions` is gone and `e3b` carries `isDefault: true`, so exactly
one branch runs.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01Q8as8yR67v41xEdomiTba9
* docs(automation): name the #4414 shape in the branching section
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01Q8as8yR67v41xEdomiTba9
* docs(spec): regenerate the flow/decision reference pages after the isDefault contract change
`content/docs/references/` is generated from `packages/spec`; the #4414
`.describe()` rewrites left flow.mdx and schemaless-node-config.mdx stale,
which is what `check:docs` caught.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01Q8as8yR67v41xEdomiTba9
---------
Co-authored-by: Claude <noreply@anthropic.com>
fix(automation): a decision's three declared ways to route a branch are now one working model (#4414)
9
+
10
+
A `decision` node advertised three mechanisms for splitting a path and only one
11
+
of them did anything. The other two were the ADR-0049 `declared ≠ enforced`
12
+
shape, and the pair of them shipped a guard that does not guard in
13
+
`examples/app-crm`.
14
+
15
+
| mechanism | before | now |
16
+
|:---|:---|:---|
17
+
|`edge.condition`| ✅ the only one that worked | unchanged |
18
+
|`edge.isDefault`|**zero readers** anywhere but the schema declaration | BPMN default flow, enforced in `traverseNext`|
19
+
|`decision.config.conditions[].label` → `branchLabel`| matched **0** out-edge labels across every example app, then fell back to the full edge set in silence | routes; an unclaimable label is logged, not swallowed |
20
+
21
+
## What was broken, end to end
22
+
23
+
`crm_convert_lead_wizard` means "already converted → abort screen; otherwise →
24
+
the wizard". It ran **both**: an already-converted lead got
25
+
"This lead has already been converted" and then walked straight into the
26
+
conversion wizard behind it. Four independent silences stacked up:
27
+
28
+
1. the decision's first condition was authored `{lead_record.status} ==
29
+
'converted'` — braces in a slot declared bare CEL, so it was string-compared
30
+
and never true;
31
+
2. the second (`'true'`) therefore won, yielding `branchLabel: 'No — proceed'`;
32
+
3. no out-edge carried that label (they were `'Yes'` / `'No'`), so traversal
33
+
discarded the branch and considered every out-edge;
34
+
4.`e3b` was unconditional, so it ran regardless — and the natural fix, marking
35
+
it `isDefault: true`, was a dead key.
36
+
37
+
## The model
38
+
39
+
`branchLabel` narrows the edge set → `condition` gates each edge → `isDefault`
40
+
catches whatever is left. Concretely:
41
+
42
+
-**`isDefault` is enforced.** A default edge is traversed only when no
43
+
conditional sibling of the same source node matched, and it is no longer part
44
+
of the unconditional parallel fan-out — that distinction is the whole point of
45
+
the marker. Passed over because a real branch won, its target records the same
46
+
`skipped` step a closed gate does (#4354).
47
+
-**An unclaimable branch label warns.** Traversal still falls back to the full
48
+
edge set (a run mid-flight must not die on a metadata error) but says so,
49
+
naming the computed branch and the out-edge labels that exist.
50
+
-**A decision that declares no `conditions` reports no branch.** It used to
51
+
report `'default'` unconditionally — a label no out-edge in the repo ever
52
+
carried — which is why every decision node fell back to the full edge set.
53
+
The `'default'` sentinel survives for the case it actually describes (declared
54
+
conditions, none matched) and is now claimed by the `isDefault` edge as well
55
+
as by an edge literally labelled `'default'`.
56
+
-**`conditions[].expression` is evaluated as the bare CEL it is declared to
57
+
be.** The raw string went to the legacy `{var}` template path, where
58
+
`lead.status == 'converted'` cannot resolve and the branch is decided by
59
+
string comparison. Unlike `edge.condition` this slot carries no
60
+
`ExpressionInput` envelope — the decision descriptor is deliberately
61
+
schemaless — so the executor supplies the dialect. A brace-in-CEL predicate
62
+
now fails loudly (ADR-0032 §1c) instead of deciding `false`.
63
+
64
+
## Caught at authoring time too
65
+
66
+
Four new `os build` / `os validate` warnings, because a wrong route is silent at
67
+
run time by nature (Prime Directive #12):
68
+
69
+
`flow-branch-label-unmatched` (the shipped shape),
70
+
`flow-decision-unconditional-branch` (a guarded decision with an unconditional
71
+
sibling — the actual hole), `flow-default-edge-with-condition` and
72
+
`flow-multiple-default-edges`.
73
+
74
+
Both of the first two fire on the pre-fix `convert-lead.flow.ts` and are silent
75
+
after it.
76
+
77
+
## The example app
78
+
79
+
`crm_convert_lead_wizard`'s guard is now a plain exclusive gateway: the
80
+
redundant `config.conditions` is gone and `e3b` carries `isDefault: true`. One
81
+
mechanism per decision, and exactly one branch runs.
82
+
83
+
Verified: 11 new engine/executor tests (including the reported repro in both
84
+
directions), 12 new linter tests; `@objectstack/service-automation` 577 tests
85
+
and `@objectstack/cli` 652 tests green, all three example apps build with no new
Copy file name to clipboardExpand all lines: content/docs/automation/flows.mdx
+55-1Lines changed: 55 additions & 1 deletion
Display the source diff
Display the rich diff
Original file line number
Diff line number
Diff line change
@@ -731,7 +731,61 @@ Edges connect nodes and define the execution path:
731
731
|`type`|`enum`| optional |`'default'` (success), `'fault'` (error), `'conditional'` (expression-guarded), or `'back'` (declared back-edge, ADR-0044); defaults to `'default'`|
732
732
|`condition`|`string`| optional | Boolean CEL predicate for branching (a bare string is stored as `{ dialect: 'cel', source }`) |
733
733
|`label`|`string`| optional | Label displayed on the connector — cosmetic only. It does **not** select a path except on a branching node (`decision` / `approval`), which picks its out-edge by label. |
734
-
|`isDefault`|`boolean`| optional | BPMN default-flow marker (interop). Accepted by the schema, but **the engine does not read it** — traversal selects by `condition` and by `label`. On a `decision` node, the fallback is the out-edge labelled `default`: when no `conditions[]` entry matches, the node emits `branchLabel: 'default'`|
734
+
|`isDefault`|`boolean`| optional | BPMN default flow — the **"otherwise" branch**. Traversed only when no sibling `condition` on the same source node matched; never part of the unconditional parallel fan-out. Mutually exclusive with `condition`, at most one per node |
735
+
736
+
### Branching — pick one mechanism per node
737
+
738
+
A node has exactly two ways to split its path, and mixing them is what makes a
739
+
guard stop guarding (#4414).
740
+
741
+
**Branch on the edges** (BPMN exclusive gateway — the default choice):
|**type**|`Enum<'default' \| 'fault' \| 'conditional' \| 'back'>`| optional | Connection type: default (normal flow), fault (error path), conditional (expression-guarded), or back (ADR-0044 declared back-edge — traversed normally at run time, but excluded from DAG cycle validation so a revise/rework loop can re-enter an earlier node) |
86
86
|**label**|`string`| optional | Label on the connector |
87
-
|**isDefault**|`boolean`| optional |Marks this edge as the default path when no other conditions match|
87
+
|**isDefault**|`boolean`| optional |BPMN default flow: traverse this edge only when no sibling conditional edge of the same source node matched. Mutually exclusive with `condition`; at most one per source node.|
Copy file name to clipboardExpand all lines: content/docs/references/automation/schemaless-node-config.mdx
+1-1Lines changed: 1 addition & 1 deletion
Display the source diff
Display the rich diff
Original file line number
Diff line number
Diff line change
@@ -121,7 +121,7 @@ const result = DecisionCondition.parse(data);
121
121
122
122
| Property | Type | Required | Description |
123
123
| :--- | :--- | :--- | :--- |
124
-
|**label**|`string`| ✅ | Branch label; the winning branch resumes down the out-edge with this label ('true' expression = default/else path) |
124
+
|**label**|`string`| ✅ | Branch label; the winning branch resumes down the out-edge with this label (no match → the out-edge marked isDefault, or one labelled 'default') |
125
125
|**expression**|`string`| ✅ | Bare CEL predicate deciding this branch |
0 commit comments