Skip to content

Commit 104d181

Browse files
os-zhuangclaude
andauthored
fix(studio): flow wait-node inspector tolerates the loose config shape (#1974)
The wait inspector read only `waitEventConfig.{eventType,signalName,…}`, but the engine also accepts the looser `config.{eventType,…}` shape that the canonical showcase_budget_approval (and AI-authored flows) use — so a showcase-shaped wait node showed blank fields in the designer. Flow config fields gain `fallbackPath`: tolerant read (loose shape displays + dependent fields reveal), canonical write that prunes the fallback (migrate-on-edit), and the fallback config key suppressed from Advanced. The `wait` fields fall back to `config.*`. Designer now matches engine tolerance. Refs #1954 (ADR-0044). Co-authored-by: Claude Opus 4.8 <noreply@anthropic.com>
1 parent 41c60c4 commit 104d181

4 files changed

Lines changed: 82 additions & 12 deletions

File tree

Lines changed: 18 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,18 @@
1+
---
2+
"@object-ui/app-shell": patch
3+
---
4+
5+
fix(studio): flow wait-node inspector tolerates the loose `config` shape
6+
7+
The wait-node property form read only the spec-canonical
8+
`waitEventConfig.{eventType,signalName,…}`, but the engine also accepts a looser
9+
`config.{eventType,…}` shape — which the canonical `showcase_budget_approval`
10+
(and AI-authored flows) use. So a showcase-shaped wait node opened in the
11+
designer showed blank "Wait for" / "Signal name" fields.
12+
13+
Flow config fields gain an optional `fallbackPath`: reads fall back to it (so
14+
loose-shape wait nodes display, and dependent fields reveal), writes target the
15+
canonical path and prune the fallback (migrate-on-edit), and the fallback's
16+
config key is suppressed from the Advanced block. The `wait` fields now fall
17+
back to `config.*`, so the designer matches the engine's tolerance. Pairs with
18+
the ADR-0044 revise-loop authoring (#1954).

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

Lines changed: 11 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -30,6 +30,7 @@ import {
3030
getFieldValue,
3131
configKeyOf,
3232
FLOW_NODE_TYPE_OPTIONS,
33+
type FlowConfigField,
3334
} from './flow-node-config';
3435
import { jsonSchemaToFlowFields } from './json-schema-to-fields';
3536
import { useActionConfigSchemas } from '../previews/useFlowNodePalette';
@@ -157,6 +158,9 @@ export function FlowNodeInspector({ selection, draft, onPatch, onClearSelection,
157158
for (const f of fields) {
158159
const k = configKeyOf(f);
159160
if (k) s.add(k);
161+
// A loose-shape fallback rooted at `config` is claimed too, so a tolerated
162+
// legacy key (e.g. a wait node's `config.eventType`) never leaks to Advanced.
163+
if (f.fallbackPath && f.fallbackPath.length >= 2 && f.fallbackPath[0] === 'config') s.add(f.fallbackPath[1]);
160164
}
161165
return s;
162166
}, [fields]);
@@ -196,8 +200,12 @@ export function FlowNodeInspector({ selection, draft, onPatch, onClearSelection,
196200
// Screen nodes (and the `user_task` alias) get a live end-user preview.
197201
const isScreen = node.type === 'screen' || node.type === 'user_task';
198202

199-
const setField = (path: string[], value: unknown) => {
200-
const nextNode = setAtPath(node, path, value);
203+
const setField = (field: FlowConfigField, value: unknown) => {
204+
const path = field.path;
205+
let nextNode = setAtPath(node, path, value);
206+
// Migrate-on-edit: writing the canonical path drops any looser fallback
207+
// location, so the node never carries a stale duplicate (engine + designer agree).
208+
if (field.fallbackPath) nextNode = setAtPath(nextNode, field.fallbackPath, undefined);
201209
const patch: Record<string, unknown> = { nodes: spliceArray(nodes, index, nextNode) };
202210
// Decision branches drive routing — mirror them onto the node's outgoing
203211
// edges so the engine/simulator can actually branch (they read
@@ -285,7 +293,7 @@ export function FlowNodeInspector({ selection, draft, onPatch, onClearSelection,
285293
key={field.id}
286294
field={field}
287295
value={getFieldValue(node, field)}
288-
onCommit={(v) => setField(field.path, v)}
296+
onCommit={(v) => setField(field, v)}
289297
disabled={readOnly}
290298
locale={locale}
291299
context={{ draft, node }}

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

Lines changed: 28 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,7 @@
11
// Copyright (c) 2025 ObjectStack. Licensed under the Apache-2.0 license.
22

33
import { describe, it, expect } from 'vitest';
4-
import { fieldsForNodeType, isFieldVisible } from './flow-node-config';
4+
import { fieldsForNodeType, isFieldVisible, getFieldValue } from './flow-node-config';
55

66
describe('start node trigger-field gating (#5)', () => {
77
const fields = fieldsForNodeType('start');
@@ -49,3 +49,30 @@ describe('approval node config (ADR-0044)', () => {
4949
expect(isFieldVisible(maxRevisions!, { id: 'a', type: 'approval' }, fields)).toBe(true);
5050
});
5151
});
52+
53+
54+
describe('wait node loose-config fallback (ADR-0044 showcase parity)', () => {
55+
const fields = fieldsForNodeType('wait');
56+
const eventType = fields.find((f) => f.id === 'waitEventConfig.eventType')!;
57+
const signalName = fields.find((f) => f.id === 'waitEventConfig.signalName')!;
58+
59+
it('reads the canonical waitEventConfig shape', () => {
60+
const node = { id: 'w', type: 'wait', waitEventConfig: { eventType: 'signal', signalName: 'x' } };
61+
expect(getFieldValue(node, eventType)).toBe('signal');
62+
expect(getFieldValue(node, signalName)).toBe('x');
63+
});
64+
65+
it('falls back to a loose config shape the engine also accepts', () => {
66+
// showcase_budget_approval authors the wait node as `config: { eventType, signalName }`.
67+
const node = { id: 'w', type: 'wait', config: { eventType: 'signal', signalName: 'budget_revision' } };
68+
expect(getFieldValue(node, eventType)).toBe('signal');
69+
expect(getFieldValue(node, signalName)).toBe('budget_revision');
70+
// The dependent field reveals because its controller resolves via the fallback.
71+
expect(isFieldVisible(signalName, node, fields)).toBe(true);
72+
});
73+
74+
it('prefers the canonical path when both shapes are present', () => {
75+
const node = { id: 'w', type: 'wait', waitEventConfig: { eventType: 'timer' }, config: { eventType: 'signal' } };
76+
expect(getFieldValue(node, eventType)).toBe('timer');
77+
});
78+
});

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

Lines changed: 25 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -118,6 +118,14 @@ export interface FlowConfigField {
118118
* the spec's top-level `node.waitEventConfig.eventType`.
119119
*/
120120
path: string[];
121+
/**
122+
* Optional secondary read location used when `path` holds no value — lets the
123+
* inspector tolerate a looser on-disk shape the engine also accepts (e.g. a
124+
* `wait` node authored with `config.eventType` instead of the spec-canonical
125+
* `waitEventConfig.eventType`). Reads fall back to it; the inspector writes the
126+
* canonical `path` and prunes the fallback (migrate-on-edit).
127+
*/
128+
fallbackPath?: string[];
121129
/** Human-readable field label (English — repo is English-only). */
122130
label: string;
123131
kind: FlowConfigFieldKind;
@@ -471,23 +479,27 @@ const FLOW_NODE_CONFIG: Record<string, FlowConfigField[]> = {
471479
{ value: 'condition', label: 'Condition' },
472480
],
473481
defaultValue: 'timer',
482+
fallbackPath: ['config', 'eventType'],
474483
}),
475484
at('waitEventConfig', 'timerDuration', 'Duration', 'text', {
476485
placeholder: 'PT1H · P3D',
477486
help: 'ISO 8601 duration (e.g. PT1H, P3D).',
478487
showWhen: { field: 'waitEventConfig.eventType', equals: ['timer'] },
488+
fallbackPath: ['config', 'timerDuration'],
479489
}),
480490
at('waitEventConfig', 'signalName', 'Signal name', 'text', {
481491
placeholder: 'contract.renewed',
482492
showWhen: { field: 'waitEventConfig.eventType', equals: ['signal', 'webhook'] },
493+
fallbackPath: ['config', 'signalName'],
483494
}),
484-
at('waitEventConfig', 'timeoutMs', 'Timeout (ms)', 'number', { placeholder: '3600000' }),
495+
at('waitEventConfig', 'timeoutMs', 'Timeout (ms)', 'number', { placeholder: '3600000', fallbackPath: ['config', 'timeoutMs'] }),
485496
at('waitEventConfig', 'onTimeout', 'On timeout', 'select', {
486497
options: [
487498
{ value: 'fail', label: 'Fail' },
488499
{ value: 'continue', label: 'Continue' },
489500
],
490501
defaultValue: 'fail',
502+
fallbackPath: ['config', 'onTimeout'],
491503
}),
492504
],
493505
subflow: [
@@ -595,14 +607,19 @@ export function fieldsForNodeType(type?: string): FlowConfigField[] {
595607
return FLOW_NODE_CONFIG[canonical] ?? [];
596608
}
597609

598-
/** Read the current value at a field's node path. */
610+
/** Read the current value at a field's node path, falling back to `fallbackPath`. */
599611
export function getFieldValue(node: Record<string, unknown> | null | undefined, field: FlowConfigField): unknown {
600-
let cur: unknown = node;
601-
for (const seg of field.path) {
602-
if (cur && typeof cur === 'object' && !Array.isArray(cur)) cur = (cur as Record<string, unknown>)[seg];
603-
else return undefined;
604-
}
605-
return cur;
612+
const read = (path: string[]): unknown => {
613+
let cur: unknown = node;
614+
for (const seg of path) {
615+
if (cur && typeof cur === 'object' && !Array.isArray(cur)) cur = (cur as Record<string, unknown>)[seg];
616+
else return undefined;
617+
}
618+
return cur;
619+
};
620+
const primary = read(field.path);
621+
if (primary !== undefined) return primary;
622+
return field.fallbackPath ? read(field.fallbackPath) : undefined;
606623
}
607624

608625
/**

0 commit comments

Comments
 (0)