Skip to content

Commit efd7767

Browse files
os-zhuangclaude
andauthored
fix(flow-designer): the default path is the edge marker, not the branch (#3148)
Two help strings on the Decision inspector that described mechanisms the engine does not have. The Branches editor said a branch whose expression is "true" IS the default/else path. It is how you ASK for one: FlowEdgeInspector.applyBranch() turns such a branch into `isDefault: true` on the out-edge it wires, and the marker on that edge is what routes. Conflating the two is the reading that let framework#4414 ship a decision whose guard did not guard — worth being exact about now that `isDefault` is actually enforced (the key had zero readers until then, so this designer was writing a marker nothing honoured). Also states that branches are tried in order and that the expression is bare CEL, since a braced predicate is now a build failure (framework#4439). The legacy single `Condition` field said "Prefer Branches above", which reads as "this works, but the other is better". It does not work at all: the decision executor never reads `config.condition`. The engine only honours that key on a Start node, as the trigger gate; framework#4414 added `flow-inert-node-condition` to `os validate` for exactly this. The field stays render-only (`__legacy__` never matches) so a stored value is not invisible, but the help now says it is inert and where to move the predicate. Claude-Session: https://claude.ai/code/session_01Q8as8yR67v41xEdomiTba9 Co-authored-by: Claude <noreply@anthropic.com>
1 parent 5eaa861 commit efd7767

2 files changed

Lines changed: 43 additions & 2 deletions

File tree

Lines changed: 29 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,29 @@
1+
---
2+
"@object-ui/app-shell": patch
3+
---
4+
5+
Say what the Decision inspector actually does: the default path is the edge marker, not the branch.
6+
7+
Two help strings described mechanisms the engine does not have.
8+
9+
The **Branches** editor said a branch whose expression is `"true"` *is* the
10+
default/else path. It is how you **ask** for one — `FlowEdgeInspector.applyBranch()`
11+
turns such a branch into `isDefault: true` on the out-edge it wires, and the marker
12+
on that edge is what routes. Conflating the two is the reading that let
13+
objectstack-ai/objectstack#4414 ship a decision whose guard did not guard, and it is
14+
worth being exact about now that `isDefault` is finally enforced: the key had **zero
15+
readers** in the engine until then, so this designer had been writing a marker
16+
nothing honoured, and every Studio "default/else" edge ran unconditionally alongside
17+
whichever branch matched. The help also now states that branches are tried in order
18+
and that the expression is bare CEL — a braced predicate there is a build failure
19+
since objectstack-ai/objectstack#4439.
20+
21+
The legacy single **Condition** field said *"Prefer Branches above"*, which reads as
22+
"this works, but the other is better". It does not work at all: the decision executor
23+
never reads `config.condition`. The engine honours that key only on a Start node, as
24+
the trigger gate, and `os validate` now reports it as `flow-inert-node-condition`.
25+
The field stays render-only (its `__legacy__` controller never matches, so it is not
26+
offered for new authoring) so a stored value is not invisible — but the help says it
27+
is inert and where the predicate belongs instead.
28+
29+
Text only; no behaviour change on this side.

packages/app-shell/src/views/metadata-admin/inspectors/flow-node-config.ts

Lines changed: 14 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -407,7 +407,12 @@ const FLOW_NODE_CONFIG: Record<string, FlowConfigField[]> = {
407407
],
408408
decision: [
409409
cfg('conditions', 'Branches', 'objectList', {
410-
help: 'Each branch has a label, a CEL expression, and a target node. A branch whose expression is "true" is the default/else path. Picking a target wires the branch’s outgoing edge (creating or updating it); clearing it detaches that edge.',
410+
// framework#4414: the default/else path is the out-edge marked
411+
// `isDefault`, not the branch itself — a `true` branch is how you ask for
412+
// one, and FlowEdgeInspector.applyBranch() writes the marker. Saying
413+
// "a `true` branch IS the default path" conflated the two, which is the
414+
// reading that let a decision ship with a guard that did not guard.
415+
help: 'Each branch has a label, a CEL expression (no {braces}), and a target node. Branches are tried in order and the first match wins. A branch whose expression is "true" is the catch-all: picking its target marks that out-edge as the default path, taken only when no other branch matched. Picking a target wires the branch’s outgoing edge (creating or updating it); clearing it detaches that edge.',
411416
columns: [
412417
{ key: 'label', label: 'Label', kind: 'text', placeholder: 'Has deals' },
413418
{ key: 'expression', label: 'Expression', kind: 'expression', placeholder: 'expiring_deals.length > 0' },
@@ -416,9 +421,16 @@ const FLOW_NODE_CONFIG: Record<string, FlowConfigField[]> = {
416421
{ key: 'target', label: 'Target', kind: 'reference', placeholder: 'next node', ref: { kind: 'node' } },
417422
],
418423
}),
424+
// Render-only for a stored legacy value (`__legacy__` never matches, so it
425+
// is not offered for new authoring). The old help said "Prefer Branches
426+
// above", which reads as "this works, but the other is better" — it does
427+
// not work at all: framework#4414 confirmed the decision executor never
428+
// reads `config.condition`. The key is the trigger gate on a `start` node
429+
// and inert on every other node type, and `os validate` now reports it as
430+
// `flow-inert-node-condition`.
419431
cfg('condition', 'Condition (single)', 'expression', {
420432
placeholder: 'amount > 10000',
421-
help: 'Legacy single-branch condition (CEL). Prefer Branches above; per-edge conditions also live on the outgoing edges.',
433+
help: 'Inert — nothing reads this. The engine only honours `condition` on a Start node (the trigger gate); on a Decision it gates nothing. Shown so a stored value is not invisible. Move the predicate to a branch above, or to the outgoing edge’s own condition, then clear this.',
422434
showWhen: { field: '__legacy__', equals: [] },
423435
}),
424436
],

0 commit comments

Comments
 (0)