Skip to content

Commit 5a89ee5

Browse files
authored
feat(flow-designer): #2670 Phase 3 — nested container node selection + schema-driven editing (#2699)
Select and edit a node inside a loop/parallel/try_catch region through the same schema-driven inspector as a top-level node, with path write-back into config.<region>.nodes[i]. Adds the xExpression marker + expression/template validation gating. Verified: 865 unit tests, type-check, eslint, and browser E2E across all three container types (loop/parallel/try_catch) against the showcase flow shapes. Companion descriptor-side work: objectstack-ai/objectstack#3304.
1 parent 675bf8b commit 5a89ee5

22 files changed

Lines changed: 1530 additions & 106 deletions
Lines changed: 36 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,36 @@
1+
---
2+
"@object-ui/app-shell": minor
3+
---
4+
5+
feat(studio): select and edit nested container nodes through the schema-driven flow inspector (#2670)
6+
7+
Phase 2 (#2680) expanded a container's regions (`loop.body` /
8+
`parallel.branches[]` / `try_catch.try`/`catch`) inline on the flow designer
9+
canvas, but the nested nodes were read-only — changing one still meant editing
10+
the container's Advanced JSON by hand. A nested node is now a first-class
11+
selection: click it on the expanded canvas and it opens in the SAME
12+
schema-driven inspector as a top-level node, with a `container › region › node`
13+
breadcrumb. Edits (label / type / description / typed config fields / Advanced
14+
JSON) write straight back into `config.<region>.nodes[i]` — the write rebuilds
15+
the container with explicit spreads so the `config.branches` array stays an
16+
array and each region's own `edges` / a branch's `name` are preserved.
17+
18+
Scope resolves correctly for the region's outer context (ADR-0031): a loop
19+
body node sees the loop's `iteratorVariable` in its data picker even though the
20+
container's own outputs are otherwise out of scope at its id.
21+
22+
This phase is edit-only by design. A nested node keeps its id read-only (rename
23+
it in the container's Advanced JSON), has no delete, and — for a nested
24+
decision — drops the virtual Target column, since a region sub-graph's internal
25+
routing is not managed by the inspector yet (nested region-edge editing,
26+
structural add/remove, and drag are follow-ups). A stale nested deep link
27+
(the draft moved on) resolves to a harmless empty-state rather than writing to
28+
the wrong node.
29+
30+
Also fixes an expression/template validation split now that the engine
31+
publishes a loop `configSchema`: a string property can carry an `xExpression:
32+
'expression' | 'template'` marker so the designer renders bare-CEL vs
33+
`interpolate()` `{var}` semantics (mono editor, data-picker brace mode, and
34+
whether the CEL brace-trap applies) instead of guessing from the field name. A
35+
loop / map `collection` (`{leadList}`) is a template, so it no longer
36+
false-positives as a malformed condition inline or on the canvas badge.

apps/console/src/preview-samples.ts

Lines changed: 25 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -118,6 +118,27 @@ export const SAMPLES: Record<string, Record<string, unknown>> = {
118118
nodes: [
119119
{ id: 'start', type: 'start', label: 'Schedule (daily)', config: { triggerType: 'schedule', objectName: 'contract', condition: 'status == "active"', cron: '0 9 * * *' } },
120120
{ id: 'find', type: 'get_record', label: 'Find expiring contracts', config: { objectName: 'contract', filter: { status: 'active' }, outputVariable: 'contracts' } },
121+
// ADR-0031 structured container: a loop whose `config.body` region is a
122+
// nested sub-graph. On the designer canvas it expands inline (#2680) and
123+
// its body nodes are selectable + editable through the same schema-driven
124+
// inspector as top-level nodes (#2670 Phase 3).
125+
{
126+
id: 'each_contract',
127+
type: 'loop',
128+
label: 'For each expiring contract',
129+
config: {
130+
collection: '{contracts}',
131+
iteratorVariable: 'contract',
132+
body: {
133+
nodes: [
134+
{ id: 'charge_fee', type: 'http_request', label: 'Charge renewal fee', config: { method: 'POST', url: 'https://api.example.com/billing/charge', outputVariable: 'charge' } },
135+
],
136+
edges: [],
137+
},
138+
},
139+
},
140+
{ id: 'fan_out', type: 'parallel', label: 'Notify in parallel', config: { branches: [ { name: 'Email the owner', nodes: [ { id: 'email_owner', type: 'script', label: 'Email Owner', config: { actionType: 'email', template: 'renewal_reminder', recipients: ['owner.email'] } } ], edges: [] }, { name: 'Post to Slack', nodes: [ { id: 'slack_post', type: 'script', label: 'Slack Notify', config: { actionType: 'slack' } } ], edges: [] } ] } },
141+
{ id: 'guard', type: 'try_catch', label: 'Push with retry', config: { errorVariable: '$error', try: { nodes: [ { id: 'push', type: 'http_request', label: 'Push to CRM', config: { method: 'POST', url: 'https://api.example.com/v1/tasks' } } ], edges: [] }, catch: { nodes: [ { id: 'record_failure', type: 'update_record', label: 'Flag Sync Failure', config: { objectName: 'contract', fields: { reminded: false } } } ], edges: [] } } },
121142
{ id: 'check', type: 'decision', label: 'Within reminder window?', config: { conditions: [ { label: 'Within window', expression: 'daysToExpiry <= daysBefore' }, { label: 'Else', expression: 'true' } ] } },
122143
{ id: 'email', type: 'connector_action', label: 'Send renewal email', connectorConfig: { connectorId: 'email', actionId: 'send', input: { to: 'owner.email', template: 'renewal_reminder' } } },
123144
{ id: 'wait', type: 'wait', label: 'Wait 3 days', waitEventConfig: { eventType: 'timer', timerDuration: 'P3D', onTimeout: 'continue' } },
@@ -130,7 +151,10 @@ export const SAMPLES: Record<string, Record<string, unknown>> = {
130151
],
131152
edges: [
132153
{ source: 'start', target: 'find' },
133-
{ source: 'find', target: 'check' },
154+
{ source: 'find', target: 'each_contract' },
155+
{ source: 'each_contract', target: 'fan_out' },
156+
{ source: 'fan_out', target: 'guard' },
157+
{ source: 'guard', target: 'check' },
134158
{ source: 'check', target: 'email', condition: 'daysToExpiry <= daysBefore' },
135159
{ source: 'check', target: 'skip', isDefault: true },
136160
{ source: 'email', target: 'wait' },

packages/app-shell/src/views/metadata-admin/i18n.ts

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -338,6 +338,7 @@ const ENGINE_STRINGS_EN: Record<string, string> = {
338338
'engine.inspector.flowNode.advanced': 'Advanced (JSON)',
339339
'engine.inspector.flowNode.advancedHint': 'Optional custom keys not covered by the form above — most flows don\u2019t need this.',
340340
'engine.inspector.flowNode.noConfig': 'No configuration needed for this node type.',
341+
'engine.inspector.flowNode.nestedIdHint': 'A node inside a container region keeps its id here — rename it in the container’s Advanced JSON.',
341342
'engine.inspector.flowNode.kv.add': 'Add entry',
342343
'engine.inspector.flowNode.kv.key': 'Key',
343344
'engine.inspector.flowNode.kv.value': 'Value',
@@ -1715,6 +1716,7 @@ const ENGINE_STRINGS_ZH: Record<string, string> = {
17151716
'engine.inspector.flowNode.advanced': '高级(JSON)',
17161717
'engine.inspector.flowNode.advancedHint': '上方表单未覆盖的可选自定义键 —— 大多数流程无需填写。',
17171718
'engine.inspector.flowNode.noConfig': '此节点类型无需配置。',
1719+
'engine.inspector.flowNode.nestedIdHint': '容器区域内的节点 ID 在此只读 —— 请在容器的高级 JSON 中重命名。',
17181720
'engine.inspector.flowNode.kv.add': '添加条目',
17191721
'engine.inspector.flowNode.kv.key': '键',
17201722
'engine.inspector.flowNode.kv.value': '值',
Lines changed: 102 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,102 @@
1+
// Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license.
2+
3+
/**
4+
* FlowNodeConfigField — inline expression/template validation gating (#2670
5+
* Phase 3 B). The malformed-condition brace-trap (ADR-0032) must fire on a real
6+
* CEL predicate but stay silent on a legitimate `interpolate()` `{var}` template
7+
* — an expression field flagged `refMode: 'template'` (e.g. a loop/map
8+
* collection). The scope-aware unknown-reference note still applies to a
9+
* template, using `{…}`-hole semantics rather than CEL predicate semantics.
10+
*/
11+
12+
import { describe, it, expect, afterEach } from 'vitest';
13+
import { render, screen, cleanup } from '@testing-library/react';
14+
import { FlowNodeConfigField } from './FlowNodeConfigField';
15+
import type { FlowConfigField } from './flow-node-config';
16+
import type { ScopeGroup } from './useFlowScope';
17+
18+
afterEach(cleanup);
19+
20+
/** A minimal scope with the given variable tokens (all in the `variables` group). */
21+
function scope(tokens: string[]): ScopeGroup[] {
22+
return [
23+
{
24+
id: 'variables',
25+
label: 'Flow variables',
26+
refs: tokens.map((token) => ({ token, label: token, group: 'variables' as const })),
27+
},
28+
];
29+
}
30+
31+
const TEMPLATE_COLLECTION: FlowConfigField = {
32+
id: 'collection',
33+
path: ['config', 'collection'],
34+
label: 'Collection',
35+
kind: 'expression',
36+
refMode: 'template',
37+
};
38+
39+
const CEL_CONDITION: FlowConfigField = {
40+
id: 'condition',
41+
path: ['config', 'condition'],
42+
label: 'Condition',
43+
kind: 'expression',
44+
};
45+
46+
describe('FlowNodeConfigField — expression vs template validation gating', () => {
47+
it('does NOT flag a `{leadList}` template on an expression field in template mode', () => {
48+
render(
49+
<FlowNodeConfigField
50+
field={TEMPLATE_COLLECTION}
51+
value="{leadList}"
52+
onCommit={() => {}}
53+
scopeGroups={scope(['leadList'])}
54+
/>,
55+
);
56+
// The pre-fix bug: the CEL brace-trap fired on the legal single-brace hole.
57+
expect(screen.queryByRole('alert')).toBeNull();
58+
// In scope → no unknown-reference note either.
59+
expect(screen.queryByRole('note')).toBeNull();
60+
});
61+
62+
it('surfaces an out-of-scope hole as a template NOTE (not a CEL error)', () => {
63+
render(
64+
<FlowNodeConfigField
65+
field={TEMPLATE_COLLECTION}
66+
value="{leadLst}"
67+
onCommit={() => {}}
68+
scopeGroups={scope(['leadList'])}
69+
/>,
70+
);
71+
// No brace-trap error…
72+
expect(screen.queryByRole('alert')).toBeNull();
73+
// …but a gentle "did you mean" note, scanned as a `{…}` template hole.
74+
const note = screen.getByRole('note');
75+
expect(note.textContent).toMatch(/leadList/);
76+
});
77+
78+
it('still flags a genuine `{record.x}` brace-in-CEL mistake on a predicate field', () => {
79+
render(
80+
<FlowNodeConfigField
81+
field={CEL_CONDITION}
82+
value="{record.amount} > 10"
83+
onCommit={() => {}}
84+
scopeGroups={scope(['record'])}
85+
/>,
86+
);
87+
expect(screen.getByRole('alert')).toBeInTheDocument();
88+
});
89+
90+
it('does not flag a well-formed CEL predicate with an in-scope reference', () => {
91+
render(
92+
<FlowNodeConfigField
93+
field={CEL_CONDITION}
94+
value="amount > 10"
95+
onCommit={() => {}}
96+
scopeGroups={scope(['amount'])}
97+
/>,
98+
);
99+
expect(screen.queryByRole('alert')).toBeNull();
100+
expect(screen.queryByRole('note')).toBeNull();
101+
});
102+
});

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

Lines changed: 13 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -182,17 +182,24 @@ export function FlowNodeConfigField({ field, value, onCommit, disabled, locale,
182182

183183
// ADR-0032 — surface a malformed condition (e.g. the `{record.x}` brace-in-CEL
184184
// mistake) inline, with the same corrective message the build/agent emit. Only
185-
// for expression fields (a genuine template uses single-brace `{var}` legally).
185+
// for expression fields in a *predicate* mode — an expression field flagged
186+
// `refMode: 'template'` (e.g. a loop/map collection authored as `{leadList}`)
187+
// is an `interpolate()` single-brace template where `{var}` is legal, so the
188+
// CEL brace-trap must be gated off or it false-positives on every `{…}`.
189+
const isTemplate = refMode === 'template';
186190
const exprIssue =
187-
field.kind === 'expression' ? validateExpressionClient('predicate', value) : null;
191+
field.kind === 'expression' && !isTemplate ? validateExpressionClient('predicate', value) : null;
188192

189193
// #1934 — pair the picker with a gentle, scope-aware "unknown reference"
190-
// warning: CEL for expression fields, `{…}` holes for template fields. Skipped
191-
// for free-form code (refMode 'expression' on a textarea, e.g. a script body)
192-
// and when scope is unknown. The brace error above takes precedence.
194+
// warning: CEL for predicate expression fields, `{…}` holes for template
195+
// fields (including an expression field in template mode). Skipped for
196+
// free-form code (refMode 'expression' on a textarea, e.g. a script body) and
197+
// when scope is unknown. The brace error above takes precedence.
193198
const scopeRole: 'predicate' | 'template' | null =
194199
field.kind === 'expression'
195-
? 'predicate'
200+
? isTemplate
201+
? 'template'
202+
: 'predicate'
196203
: refMode === 'template' && (field.kind === 'text' || field.kind === 'textarea')
197204
? 'template'
198205
: null;

0 commit comments

Comments
 (0)