Skip to content

Commit 53fbf49

Browse files
authored
test(automation): pin the control-flow trio's designer forms (#4045) (#4064)
`loop` / `parallel` / `try_catch` had NO descriptor assertions — config-schemas.test.ts covered the CRUD quartet, `assignment` and `screen`, so the designer's input for the entire control-flow trio was unguarded. These pin intent, not current bytes. All three publish a form description that is deliberately SHALLOWER and LOOSER than its Zod counterpart in control-flow.zod.ts: region keys (`loop.body`, `parallel.branches[].nodes/edges`, `try_catch.try/catch`) publish `{ type: 'array' }` with NO `items`, so a sub-graph stays opaque — it is edited on the canvas, not in a property form — while the Zod side is `z.array(FlowNodeSchema)`; and `collection`/`iteratorVariable` omit the Zod-only `minLength`/`default`. That corrects a premise of #4045: these were treated as redundant copies awaiting de-duplication by a single Zod source. They are a second artifact with a different job, and the gaps are deliberate — naively generating them would nest a form for every node inside a loop body. Verified to bite: simulating a naive z.toJSONSchema swap (deep `items` under body.nodes plus minLength/default) turns both new loop tests red, naming `loop.body.nodes must stay opaque (no items)`. Tests only; empty changeset since nothing releases.
1 parent a3cb9c8 commit 53fbf49

2 files changed

Lines changed: 128 additions & 0 deletions

File tree

Lines changed: 30 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,30 @@
1+
---
2+
---
3+
4+
Tests only — no package changes, nothing to release.
5+
6+
Pins the published `configSchema` of the `loop` / `parallel` / `try_catch`
7+
descriptors, which had no assertions at all: the designer's input for the whole
8+
control-flow trio was unguarded (#4045 step A).
9+
10+
The assertions capture *intent*, not just current bytes. All three publish a form
11+
description that is deliberately **shallower and looser** than its Zod counterpart
12+
in `control-flow.zod.ts`:
13+
14+
- region keys (`loop.body`, `parallel.branches[].nodes/edges`, `try_catch.try/catch`)
15+
publish `{ type: 'array' }` with **no `items`**, so the designer treats a sub-graph
16+
as opaque — it is edited on the canvas, not in a property form. The Zod is
17+
`FlowRegionSchema` (`z.array(FlowNodeSchema)`), which would generate the entire
18+
FlowNode/FlowEdge definition nested there;
19+
- `collection` is `z.string().min(1)` and `iteratorVariable` is `.default('item')`
20+
in Zod, yet the form publishes neither `minLength` nor `default`.
21+
22+
This matters for the rest of #4045: the plan (and the issue) assumed these three
23+
were redundant copies awaiting de-duplication by a single Zod source. They are not
24+
— they are a second artifact with a different job, and the differences are
25+
deliberate. With these pinned, any move toward a generated `configSchema` either
26+
keeps them green or has to state explicitly why the designer contract changed.
27+
28+
Verified to bite: simulating what a naive `z.toJSONSchema` swap would emit (deep
29+
`items` under `body.nodes`, plus `minLength`/`default`) turns both new `loop` tests
30+
red.

packages/services/service-automation/src/builtin/config-schemas.test.ts

Lines changed: 98 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -18,10 +18,14 @@
1818
*/
1919

2020
import { describe, it, expect } from 'vitest';
21+
import { LOOP_MAX_ITERATIONS_CEILING } from '@objectstack/spec/automation';
2122
import { AutomationEngine } from '../engine.js';
2223
import { registerCrudNodes } from './crud-nodes.js';
2324
import { registerLogicNodes } from './logic-nodes.js';
2425
import { registerScreenNodes } from './screen-nodes.js';
26+
import { registerLoopNode } from './loop-node.js';
27+
import { registerParallelNode } from './parallel-node.js';
28+
import { registerTryCatchNode } from './try-catch-node.js';
2529

2630
function silentLogger() {
2731
return { info() {}, warn() {}, error() {}, debug() {}, child() { return silentLogger(); } } as any;
@@ -39,6 +43,11 @@ interface SchemaProp {
3943
enum?: unknown[];
4044
xRef?: { kind?: string };
4145
xExpression?: string;
46+
minimum?: number;
47+
maximum?: number;
48+
minItems?: number;
49+
minLength?: number;
50+
default?: unknown;
4251
}
4352

4453
function schemaOf(engine: AutomationEngine, type: string) {
@@ -99,6 +108,95 @@ describe('builtin node configSchemas — designer parity (#3304)', () => {
99108
expectKeyValueMap(schema.properties?.defaults, 'screen.defaults');
100109
});
101110

111+
/**
112+
* The control-flow trio had NO descriptor assertions at all — the designer's
113+
* input for `loop` / `parallel` / `try_catch` was unguarded, so a change to
114+
* these literals could not be noticed (#4045 step A).
115+
*
116+
* What these pin is deliberately more than "the current bytes": each of these
117+
* three publishes a form description that is **intentionally shallower and
118+
* looser than its Zod counterpart** in `control-flow.zod.ts`. That difference
119+
* is the point, and it is what a naive "one Zod source, generate the
120+
* configSchema from it" refactor would erase:
121+
*
122+
* - **Region keys are opaque.** `loop.body`, `parallel.branches[].nodes/edges`
123+
* and `try_catch.try/catch` publish `{ type: 'array' }` with **no `items`**.
124+
* The Zod is `FlowRegionSchema` — `z.array(FlowNodeSchema)` — so generating
125+
* would emit the entire FlowNode/FlowEdge definition nested here, and the
126+
* designer would try to render a property form for a sub-graph that is
127+
* edited on the CANVAS.
128+
* - **Zod-only constraints stay out of the form.** `collection` is
129+
* `z.string().min(1)` and `iteratorVariable` is `.default('item')`, yet the
130+
* form publishes neither `minLength` nor `default`.
131+
*
132+
* So these are not redundant copies pending de-duplication; they are a
133+
* different artifact with a different job (drive a form vs. validate a value),
134+
* and the assertions below are what make that intent enforced rather than
135+
* merely true. Any move toward a generated configSchema has to keep them green
136+
* or state explicitly why the designer contract changed.
137+
*/
138+
describe('control-flow trio: the form is deliberately shallower than the Zod', () => {
139+
const engine2 = new AutomationEngine(silentLogger());
140+
registerLoopNode(engine2, ctx());
141+
registerParallelNode(engine2, ctx());
142+
registerTryCatchNode(engine2, ctx());
143+
144+
/** A region body publishes an opaque array — no `items` to recurse into. */
145+
function expectOpaqueRegion(region: SchemaProp | undefined, label: string) {
146+
expect(region, label).toBeDefined();
147+
expect(region!.type, `${label}.type`).toBe('object');
148+
for (const key of ['nodes', 'edges'] as const) {
149+
const arr = region!.properties?.[key];
150+
expect(arr, `${label}.${key}`).toBeDefined();
151+
expect(arr!.type, `${label}.${key}.type`).toBe('array');
152+
// The load-bearing absence: no element schema, so the designer treats the
153+
// sub-graph as opaque instead of forms-for-every-node.
154+
expect(arr!.items, `${label}.${key} must stay opaque (no items)`).toBeUndefined();
155+
}
156+
}
157+
158+
it('loop: template collection, bounded iterations, opaque body', () => {
159+
const schema = schemaOf(engine2, 'loop');
160+
expect(schema.properties?.collection?.xExpression).toBe('template');
161+
expect(schema.properties?.maxIterations?.type).toBe('integer');
162+
expect(schema.properties?.maxIterations?.minimum).toBe(1);
163+
expect(schema.properties?.maxIterations?.maximum).toBe(LOOP_MAX_ITERATIONS_CEILING);
164+
expectOpaqueRegion(schema.properties?.body, 'loop.body');
165+
});
166+
167+
it('loop: the form does NOT carry the Zod-only string constraints', () => {
168+
// `LoopConfigSchema.collection` is `z.string().min(1)` and
169+
// `iteratorVariable` is `.default('item')`. Generating the form from that
170+
// Zod would publish `minLength: 1` / `default: 'item'`, which this form has
171+
// never advertised. Pinned so such a change is a decision, not a side effect.
172+
const schema = schemaOf(engine2, 'loop');
173+
expect(schema.properties?.collection?.minLength).toBeUndefined();
174+
expect(schema.properties?.iteratorVariable?.minLength).toBeUndefined();
175+
expect(schema.properties?.iteratorVariable?.default).toBeUndefined();
176+
});
177+
178+
it('parallel: at least two branches, each an opaque region', () => {
179+
const schema = schemaOf(engine2, 'parallel');
180+
expect(schema.properties?.branches?.type).toBe('array');
181+
expect(schema.properties?.branches?.minItems).toBe(2);
182+
expect(schema.properties?.branches?.items?.properties?.name?.type).toBe('string');
183+
expectOpaqueRegion(schema.properties?.branches?.items, 'parallel.branches[]');
184+
});
185+
186+
it('try_catch: both regions opaque, bounded retry policy', () => {
187+
const schema = schemaOf(engine2, 'try_catch');
188+
expectOpaqueRegion(schema.properties?.try, 'try_catch.try');
189+
expectOpaqueRegion(schema.properties?.catch, 'try_catch.catch');
190+
expect(schema.properties?.errorVariable?.type).toBe('string');
191+
const retry = schema.properties?.retry?.properties;
192+
expect(retry?.maxRetries?.type).toBe('integer');
193+
expect(retry?.maxRetries?.minimum).toBe(0);
194+
expect(retry?.maxRetries?.maximum).toBe(10);
195+
expect(retry?.backoffMultiplier?.minimum).toBe(1);
196+
expect(retry?.jitter?.type).toBe('boolean');
197+
});
198+
});
199+
102200
it('decision / script stay deliberately schemaless (no partial forms)', () => {
103201
// A node with no configSchema renders identically online and offline (the
104202
// hardcoded fallback), so there is no divergence — and publishing a partial

0 commit comments

Comments
 (0)