Skip to content

Commit 47c6e25

Browse files
os-zhuangclaude
andauthored
fix(studio/flow): wire decision branches to edges, expand screen config, align simulator (#1927)
* fix(studio/flow): wire decision branches to edges, expand screen config, align simulator Found dogfooding the Studio Flow Builder as a business user: - Decision branches now mirror onto the node's outgoing edges (FlowCanvas.addNode carries the matching branch onto a new edge; FlowNodeInspector re-syncs existing edges when branches change). A decision built in Studio now routes at runtime instead of running every branch (engine/simulator branch on edge.condition). - Screen config form now edits title/description/waitForInput + object-form keys (objectName/idVariable/mode/defaults), not just fields. - Simulator applies assignment nodes (was a no-op) using the engine's shapes + {var} interpolation, and only pauses screens that collect input (engine parity). - Tests: flow-simulator screen-pause + assignment coverage updated/added. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> * fix(studio/flow): de-duplicate HTTP node in palette (canonical http) The base palette hardcoded the deprecated http_request alias while the engine publishes the canonical http, so the Add-node palette listed two HTTP entries. Base now uses http (merges with the engine descriptor into one), aliased to the http_request config group so the inspector form is unchanged. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 4.8 <noreply@anthropic.com>
1 parent 92c3242 commit 47c6e25

7 files changed

Lines changed: 253 additions & 9 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+
fix(studio/flow): wire decision branches to edges, expand screen config, align simulator with engine
6+
7+
Four fixes for the Studio Flow Builder, found dogfooding it as a business user:
8+
9+
- **Decision branches now route.** The "Branches" editor wrote `node.config.conditions`
10+
but never the outgoing edges, so a decision built entirely in Studio left every
11+
out-edge unconditional — the engine and simulator (which branch on `edge.condition`)
12+
ran *all* branches. Branches now mirror onto the node's out-edges (by order):
13+
`FlowCanvas.addNode` carries the matching branch onto a newly-connected edge, and
14+
`FlowNodeInspector` re-syncs existing edges when branches are edited (a `true`
15+
expression marks the default/else edge).
16+
- **Screen node config expanded.** The form exposed only `fields`; it now also edits
17+
`title`, `description` (interpolates `{var}`), `waitForInput`, and the object-form
18+
keys (`objectName`, `idVariable`, `mode`, `defaults`) — so a message screen or an
19+
object-form wizard step no longer requires dropping to Advanced JSON.
20+
- **Simulator applies assignment nodes.** Assignment was a no-op pass-through, so a
21+
Debug run never reflected `Set variables`. It now normalizes the same shapes the
22+
engine accepts (`assignments` map/array + flat) and interpolates `{var}`.
23+
- **Simulator screen-pause parity.** The simulator paused on every screen; it now
24+
pauses only when the screen collects input (`fields`) or sets `waitForInput`,
25+
matching the engine's `shouldPause` — a field-less screen passes through.
26+
- **Palette HTTP de-duplicated.** The base palette hardcoded the deprecated
27+
`http_request` alias while the engine publishes the canonical `http`, showing
28+
two HTTP entries. The base now uses `http` (merging into one), aliased to the
29+
`http_request` config form so the inspector is unchanged.

packages/app-shell/src/views/metadata-admin/inspectors/FlowNodeInspector.tsx

Lines changed: 59 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -44,6 +44,54 @@ interface FlowNode {
4444
[k: string]: unknown;
4545
}
4646

47+
interface FlowEdge {
48+
id?: string;
49+
source: string;
50+
target: string;
51+
condition?: unknown;
52+
label?: string;
53+
isDefault?: boolean;
54+
type?: string;
55+
[k: string]: unknown;
56+
}
57+
58+
/**
59+
* Mirror a decision node's `conditions` (branches) onto its outgoing sequence
60+
* edges, in declared order: branch i -> the i-th out-edge. A branch whose
61+
* expression is `true` marks its edge as the default/else path; an empty
62+
* expression clears the guard. Fault / back edges are left untouched.
63+
*
64+
* This is what lets a decision authored entirely in Studio actually route at
65+
* runtime: the engine and the simulator branch on `edge.condition`, NOT on
66+
* `node.config.conditions` (which is only a node-local branch list). Without
67+
* the mirror, every out-edge stays unconditional and all branches fire.
68+
*/
69+
function syncDecisionEdges(decisionId: string, conditions: unknown, edges: FlowEdge[]): FlowEdge[] {
70+
const branches = Array.isArray(conditions) ? conditions : [];
71+
let bi = 0;
72+
return edges.map((e) => {
73+
if (e.source !== decisionId || e.type === 'fault' || e.type === 'back') return e;
74+
const branch = branches[bi++] as Record<string, unknown> | undefined;
75+
if (!branch || typeof branch !== 'object') return e;
76+
const expr = typeof branch.expression === 'string' ? branch.expression.trim() : '';
77+
const label = typeof branch.label === 'string' ? branch.label.trim() : '';
78+
const next: FlowEdge = { ...e };
79+
if (label) next.label = label;
80+
else delete next.label;
81+
if (expr && expr !== 'true') {
82+
next.condition = expr;
83+
delete next.isDefault;
84+
} else if (expr === 'true') {
85+
next.isDefault = true;
86+
delete next.condition;
87+
} else {
88+
delete next.condition;
89+
delete next.isDefault;
90+
}
91+
return next;
92+
});
93+
}
94+
4795
function asConfig(node: FlowNode | null): Record<string, unknown> {
4896
const c = node?.config;
4997
return c && typeof c === 'object' && !Array.isArray(c) ? (c as Record<string, unknown>) : {};
@@ -134,7 +182,17 @@ export function FlowNodeInspector({ selection, draft, onPatch, onClearSelection,
134182

135183
const setField = (path: string[], value: unknown) => {
136184
const nextNode = setAtPath(node, path, value);
137-
onPatch({ nodes: spliceArray(nodes, index, nextNode) });
185+
const patch: Record<string, unknown> = { nodes: spliceArray(nodes, index, nextNode) };
186+
// Decision branches drive routing — mirror them onto the node's outgoing
187+
// edges so the engine/simulator can actually branch (they read
188+
// edge.condition, not node.config.conditions).
189+
if (node.type === 'decision' && path.length === 2 && path[0] === 'config' && path[1] === 'conditions') {
190+
const draftEdges = Array.isArray((draft as { edges?: unknown }).edges)
191+
? ((draft as { edges: FlowEdge[] }).edges)
192+
: [];
193+
patch.edges = syncDecisionEdges(node.id, value, draftEdges);
194+
}
195+
onPatch(patch);
138196
};
139197

140198
const commitAdvanced = () => {

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

Lines changed: 35 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -325,9 +325,19 @@ const FLOW_NODE_CONFIG: Record<string, FlowConfigField[]> = {
325325
}),
326326
{ id: 'timeoutMs', path: ['timeoutMs'], label: 'Timeout (ms)', kind: 'number', placeholder: '30000' },
327327
],
328+
// Screen — collect input (a flat `fields` list) OR render an object's full
329+
// create/edit form (`objectName`, master-detail). `title`/`description`
330+
// head the screen (description interpolates {var}); `waitForInput` forces a
331+
// pause on a field-less message/confirmation screen. All optional and shown
332+
// together so neither a message screen nor an object-form step needs JSON.
328333
screen: [
334+
cfg('title', 'Title', 'text', { placeholder: 'Request a discount', help: 'Heading shown above the screen.' }),
335+
cfg('description', 'Description', 'textarea', {
336+
placeholder: 'Enter the deal amount and the discount you want.',
337+
help: 'Body text. Interpolates {var} references (e.g. {approval_path}).',
338+
}),
329339
cfg('fields', 'Fields', 'objectList', {
330-
help: 'Fields presented on this screen.',
340+
help: 'Input fields collected on this screen. Leave empty for a message-only screen.',
331341
columns: [
332342
{ key: 'name', label: 'Name', kind: 'text', placeholder: 'discount' },
333343
{ key: 'label', label: 'Label', kind: 'text', placeholder: 'Discount %' },
@@ -336,6 +346,29 @@ const FLOW_NODE_CONFIG: Record<string, FlowConfigField[]> = {
336346
{ key: 'visibleWhen', label: 'Visible when', kind: 'expression', placeholder: 'stage == "review"' },
337347
],
338348
}),
349+
cfg('waitForInput', 'Wait for input', 'boolean', {
350+
help: 'Pause to show this screen even with no fields (a message / confirmation). A field-less screen with this off is a server pass-through.',
351+
}),
352+
cfg('objectName', 'Object form', 'reference', {
353+
ref: { kind: 'object' },
354+
placeholder: 'crm_account',
355+
help: 'Render this object\u2019s full create/edit form (incl. master-detail) instead of a flat field list.',
356+
}),
357+
cfg('idVariable', 'Saved-record variable', 'text', {
358+
placeholder: 'account_id',
359+
help: 'Object form only: variable bound to the saved record\u2019s id, for later steps.',
360+
}),
361+
cfg('mode', 'Form mode', 'select', {
362+
options: [
363+
{ value: 'create', label: 'Create' },
364+
{ value: 'edit', label: 'Edit' },
365+
],
366+
defaultValue: 'create',
367+
help: 'Object form only.',
368+
}),
369+
cfg('defaults', 'Form defaults', 'keyValue', {
370+
help: 'Object form only: prefilled values (e.g. account \u2192 {account_id}).',
371+
}),
339372
],
340373
// Approval node (ADR-0019). The node opens an approval request on entry,
341374
// suspends the run, and resumes down its `approve` / `reject` out-edge once a
@@ -526,6 +559,7 @@ const FLOW_NODE_CONFIG: Record<string, FlowConfigField[]> = {
526559
*/
527560
const TYPE_ALIASES: Record<string, string> = {
528561
action: 'legacy_action',
562+
http: 'http_request',
529563
branch: 'decision',
530564
gateway: 'decision',
531565
condition: 'decision',

packages/app-shell/src/views/metadata-admin/previews/FlowCanvas.tsx

Lines changed: 19 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -164,6 +164,25 @@ export function FlowCanvas({
164164
source: opts.from,
165165
target: id,
166166
};
167+
// When the source is a decision, carry its matching branch (by order:
168+
// the k-th out-edge takes the k-th branch) onto the new edge so it
169+
// actually routes. The decision's config.conditions are otherwise
170+
// disconnected from the edges, leaving every branch unconditional.
171+
const fromNode = nodes.find((n) => n.id === opts.from);
172+
if (fromNode?.type === 'decision') {
173+
const branches = Array.isArray(fromNode.config?.conditions)
174+
? (fromNode.config!.conditions as Array<Record<string, unknown>>)
175+
: [];
176+
const outCount = edges.filter((e) => e.source === opts.from).length;
177+
const branch = branches[outCount];
178+
if (branch && typeof branch === 'object') {
179+
const expr = typeof branch.expression === 'string' ? branch.expression.trim() : '';
180+
const label = typeof branch.label === 'string' ? branch.label.trim() : '';
181+
if (label) newEdge.label = label;
182+
if (expr === 'true') newEdge.isDefault = true;
183+
else if (expr) newEdge.condition = expr;
184+
}
185+
}
167186
patch.edges = appendArray(edges, newEdge);
168187
}
169188
onPatch(patch);

packages/app-shell/src/views/metadata-admin/previews/flow-canvas-parts.tsx

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -227,6 +227,7 @@ export function nodeTone(type: string): NodeTone {
227227
case 'delete_record':
228228
case 'get_record':
229229
return TONES.record;
230+
case 'http':
230231
case 'http_request':
231232
case 'connector_action':
232233
case 'script':
@@ -290,6 +291,7 @@ export function nodeCategory(type: string): NodeCategory {
290291
case 'screen':
291292
case 'user_task':
292293
return 'Human';
294+
case 'http':
293295
case 'http_request':
294296
case 'connector_action':
295297
case 'script':
@@ -319,7 +321,7 @@ export const NODE_PALETTE: PaletteItem[] = [
319321
{ type: 'try_catch', label: 'Try / Catch', hint: 'Protect steps with error handling and retry', category: 'Logic' },
320322
{ type: 'approval', label: 'Approval', hint: 'Pause for a human decision', category: 'Human' },
321323
{ type: 'screen', label: 'Screen', hint: 'Collect input from a user', category: 'Human' },
322-
{ type: 'http_request', label: 'HTTP request', hint: 'Call an external API', category: 'Integration' },
324+
{ type: 'http', label: 'HTTP request', hint: 'Call an external API', category: 'Integration' },
323325
{ type: 'connector_action', label: 'Connector', hint: 'Run an integration action', category: 'Integration' },
324326
{ type: 'script', label: 'Script', hint: 'Run custom code', category: 'Integration' },
325327
{ type: 'subflow', label: 'Subflow', hint: 'Invoke another flow', category: 'Flow' },
@@ -356,6 +358,7 @@ export function defaultNodeExtras(type: string): Record<string, unknown> {
356358
// Seed a node-model approval: at least one approver + spec defaults. The
357359
// author wires the out-edges with labels `approve` / `reject`.
358360
return { config: { approvers: [{ type: 'manager' }], behavior: 'first_response', lockRecord: true } };
361+
case 'http':
359362
case 'http_request':
360363
return { config: { method: 'GET' } };
361364
case 'script':

packages/app-shell/src/views/metadata-admin/previews/simulator/__tests__/flow-simulator.test.ts

Lines changed: 40 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -179,11 +179,11 @@ describe('FlowSimulator', () => {
179179
expect(sim.state.visitedNodeIds).toContain('e');
180180
});
181181

182-
it('pauses on a screen node and merges provided outputs on resume', () => {
182+
it('pauses on an input-bearing screen and merges provided outputs on resume', () => {
183183
const sim = new FlowSimulator(
184184
[
185185
{ id: 's', type: 'start' },
186-
{ id: 'scr', type: 'screen' },
186+
{ id: 'scr', type: 'screen', config: { fields: [{ name: 'discount', label: 'Discount' }] } },
187187
{ id: 'e', type: 'end' },
188188
],
189189
[
@@ -200,6 +200,44 @@ describe('FlowSimulator', () => {
200200
expect(sim.state.status).toBe('done');
201201
});
202202

203+
it('passes a field-less screen through without pausing (engine parity)', () => {
204+
const sim = new FlowSimulator(
205+
[
206+
{ id: 's', type: 'start' },
207+
{ id: 'scr', type: 'screen' },
208+
{ id: 'e', type: 'end' },
209+
],
210+
[
211+
{ source: 's', target: 'scr' },
212+
{ source: 'scr', target: 'e' },
213+
],
214+
);
215+
sim.reset();
216+
sim.runToEnd();
217+
expect(sim.state.status).toBe('done');
218+
expect(sim.state.visitedNodeIds).toContain('e');
219+
expect(sim.state.steps.find((st) => st.nodeId === 'scr')?.status).toBe('ok');
220+
});
221+
222+
it('applies assignment config (assignments map + {var} interpolation) to variables', () => {
223+
const sim = new FlowSimulator(
224+
[
225+
{ id: 's', type: 'start' },
226+
{ id: 'a', type: 'assignment', config: { assignments: { who: 'Ada', greeting: 'hi {who}' } } },
227+
{ id: 'e', type: 'end' },
228+
],
229+
[
230+
{ source: 's', target: 'a' },
231+
{ source: 'a', target: 'e' },
232+
],
233+
);
234+
sim.reset();
235+
sim.runToEnd();
236+
expect(sim.state.variables.who).toBe('Ada');
237+
expect(sim.state.variables.greeting).toBe('hi Ada');
238+
expect(sim.state.status).toBe('done');
239+
});
240+
203241
it('guards against infinite loops with a step ceiling', () => {
204242
const sim = new FlowSimulator(
205243
[

packages/app-shell/src/views/metadata-admin/previews/simulator/flow-simulator.ts

Lines changed: 67 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -28,7 +28,7 @@ import { evalCondition, validateFlowDraft } from './flow-sim-validate';
2828

2929
const MAX_STEPS = 500;
3030

31-
const PASS_THROUGH = new Set(['start', 'assignment']);
31+
const PASS_THROUGH = new Set(['start']);
3232
const MOCKED_SIDE_EFFECT = new Set([
3333
'create_record',
3434
'update_record',
@@ -161,6 +161,8 @@ export class FlowSimulator {
161161

162162
if (type === 'decision') return this.executeDecision(node);
163163

164+
if (type === 'assignment') return this.executeAssignment(node);
165+
164166
if (UNSUPPORTED.has(type)) {
165167
this.enqueueSuccessors(node);
166168
return this.record(node.id, type, node.label, 'skipped', {
@@ -182,9 +184,19 @@ export class FlowSimulator {
182184
}
183185

184186
if (type === 'screen') {
185-
this.state.status = 'paused';
186-
this.state.pausedReason = 'screen';
187-
return this.record(node.id, type, node.label, 'paused', { note: 'Screen reached — provide inputs, then continue.' });
187+
// Mirror the engine's `shouldPause`: a screen suspends only when it
188+
// collects input (`fields`) or explicitly opts in (`waitForInput`).
189+
// A field-less / `waitForInput:false` screen is a server pass-through.
190+
const fields = Array.isArray(node.config?.fields) ? (node.config!.fields as unknown[]) : [];
191+
const waitForInput = node.config?.waitForInput;
192+
const shouldPause = waitForInput === true || (fields.length > 0 && waitForInput !== false);
193+
if (shouldPause) {
194+
this.state.status = 'paused';
195+
this.state.pausedReason = 'screen';
196+
return this.record(node.id, type, node.label, 'paused', { note: 'Screen reached — provide inputs, then continue.' });
197+
}
198+
this.enqueueSuccessors(node);
199+
return this.record(node.id, type, node.label, 'ok', { note: 'Screen has no input — passed through (matches runtime).' });
188200
}
189201

190202
if (type === 'loop') {
@@ -251,6 +263,57 @@ export class FlowSimulator {
251263
});
252264
}
253265

266+
/**
267+
* assignment node — set flow variables. Normalizes the three authoring
268+
* shapes the engine accepts (Studio's `{ assignments: { var: value } }`
269+
* map, the example `{ assignments: [{ variable, value }] }` array, and the
270+
* legacy flat `{ var: value }`) and interpolates `{var}` templates — so the
271+
* Debug run mirrors runtime instead of silently no-oping.
272+
*/
273+
private executeAssignment(node: SimNode): SimStep {
274+
const cfg = node.config ?? {};
275+
const raw = cfg.assignments;
276+
const pairs: Array<[string, unknown]> = [];
277+
if (Array.isArray(raw)) {
278+
for (const item of raw) {
279+
if (item && typeof item === 'object') {
280+
const e = item as Record<string, unknown>;
281+
const name = e.variable ?? e.name ?? e.key;
282+
if (typeof name === 'string' && name) pairs.push([name, e.value]);
283+
}
284+
}
285+
} else if (raw && typeof raw === 'object') {
286+
for (const [k, v] of Object.entries(raw as Record<string, unknown>)) pairs.push([k, v]);
287+
} else {
288+
for (const [k, v] of Object.entries(cfg)) pairs.push([k, v]);
289+
}
290+
const wrote: Record<string, unknown> = {};
291+
for (const [key, value] of pairs) {
292+
const resolved = this.interpolateValue(value);
293+
this.state.variables[key] = resolved;
294+
wrote[key] = resolved;
295+
}
296+
this.enqueueSuccessors(node);
297+
return this.record(node.id, 'assignment', node.label, 'ok', {
298+
wrote: Object.keys(wrote).length ? wrote : undefined,
299+
note: Object.keys(wrote).length ? undefined : 'No assignments defined.',
300+
});
301+
}
302+
303+
/** Resolve `{var}` templates in an assignment value against live variables. */
304+
private interpolateValue(value: unknown): unknown {
305+
if (typeof value !== 'string') return value;
306+
const whole = value.match(/^\{([^}]+)\}$/);
307+
if (whole) {
308+
const v = this.state.variables[whole[1].trim()];
309+
return v !== undefined ? v : value;
310+
}
311+
return value.replace(/\{([^}]+)\}/g, (_m, k) => {
312+
const v = this.state.variables[String(k).trim()];
313+
return v === undefined || v === null ? '' : String(v);
314+
});
315+
}
316+
254317
private executeLoop(node: SimNode): SimStep {
255318
const ref = str(node.config?.collection);
256319
const iterVar = str(node.config?.iteratorVariable);

0 commit comments

Comments
 (0)