diff --git a/.changeset/pin-control-flow-designer-forms.md b/.changeset/pin-control-flow-designer-forms.md new file mode 100644 index 0000000000..31dbc95adf --- /dev/null +++ b/.changeset/pin-control-flow-designer-forms.md @@ -0,0 +1,30 @@ +--- +--- + +Tests only — no package changes, nothing to release. + +Pins the published `configSchema` of the `loop` / `parallel` / `try_catch` +descriptors, which had no assertions at all: the designer's input for the whole +control-flow trio was unguarded (#4045 step A). + +The assertions capture *intent*, not just 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 the designer treats a sub-graph + as opaque — it is edited on the canvas, not in a property form. The Zod is + `FlowRegionSchema` (`z.array(FlowNodeSchema)`), which would generate the entire + FlowNode/FlowEdge definition nested there; +- `collection` is `z.string().min(1)` and `iteratorVariable` is `.default('item')` + in Zod, yet the form publishes neither `minLength` nor `default`. + +This matters for the rest of #4045: the plan (and the issue) assumed these three +were redundant copies awaiting de-duplication by a single Zod source. They are not +— they are a second artifact with a different job, and the differences are +deliberate. With these pinned, any move toward a generated `configSchema` either +keeps them green or has to state explicitly why the designer contract changed. + +Verified to bite: simulating what a naive `z.toJSONSchema` swap would emit (deep +`items` under `body.nodes`, plus `minLength`/`default`) turns both new `loop` tests +red. diff --git a/packages/services/service-automation/src/builtin/config-schemas.test.ts b/packages/services/service-automation/src/builtin/config-schemas.test.ts index b8a9cbb163..d15ea2b88e 100644 --- a/packages/services/service-automation/src/builtin/config-schemas.test.ts +++ b/packages/services/service-automation/src/builtin/config-schemas.test.ts @@ -18,10 +18,14 @@ */ import { describe, it, expect } from 'vitest'; +import { LOOP_MAX_ITERATIONS_CEILING } from '@objectstack/spec/automation'; import { AutomationEngine } from '../engine.js'; import { registerCrudNodes } from './crud-nodes.js'; import { registerLogicNodes } from './logic-nodes.js'; import { registerScreenNodes } from './screen-nodes.js'; +import { registerLoopNode } from './loop-node.js'; +import { registerParallelNode } from './parallel-node.js'; +import { registerTryCatchNode } from './try-catch-node.js'; function silentLogger() { return { info() {}, warn() {}, error() {}, debug() {}, child() { return silentLogger(); } } as any; @@ -39,6 +43,11 @@ interface SchemaProp { enum?: unknown[]; xRef?: { kind?: string }; xExpression?: string; + minimum?: number; + maximum?: number; + minItems?: number; + minLength?: number; + default?: unknown; } function schemaOf(engine: AutomationEngine, type: string) { @@ -99,6 +108,95 @@ describe('builtin node configSchemas — designer parity (#3304)', () => { expectKeyValueMap(schema.properties?.defaults, 'screen.defaults'); }); + /** + * The control-flow trio had NO descriptor assertions at all — the designer's + * input for `loop` / `parallel` / `try_catch` was unguarded, so a change to + * these literals could not be noticed (#4045 step A). + * + * What these pin is deliberately more than "the current bytes": each of these + * three publishes a form description that is **intentionally shallower and + * looser than its Zod counterpart** in `control-flow.zod.ts`. That difference + * is the point, and it is what a naive "one Zod source, generate the + * configSchema from it" refactor would erase: + * + * - **Region keys are opaque.** `loop.body`, `parallel.branches[].nodes/edges` + * and `try_catch.try/catch` publish `{ type: 'array' }` with **no `items`**. + * The Zod is `FlowRegionSchema` — `z.array(FlowNodeSchema)` — so generating + * would emit the entire FlowNode/FlowEdge definition nested here, and the + * designer would try to render a property form for a sub-graph that is + * edited on the CANVAS. + * - **Zod-only constraints stay out of the form.** `collection` is + * `z.string().min(1)` and `iteratorVariable` is `.default('item')`, yet the + * form publishes neither `minLength` nor `default`. + * + * So these are not redundant copies pending de-duplication; they are a + * different artifact with a different job (drive a form vs. validate a value), + * and the assertions below are what make that intent enforced rather than + * merely true. Any move toward a generated configSchema has to keep them green + * or state explicitly why the designer contract changed. + */ + describe('control-flow trio: the form is deliberately shallower than the Zod', () => { + const engine2 = new AutomationEngine(silentLogger()); + registerLoopNode(engine2, ctx()); + registerParallelNode(engine2, ctx()); + registerTryCatchNode(engine2, ctx()); + + /** A region body publishes an opaque array — no `items` to recurse into. */ + function expectOpaqueRegion(region: SchemaProp | undefined, label: string) { + expect(region, label).toBeDefined(); + expect(region!.type, `${label}.type`).toBe('object'); + for (const key of ['nodes', 'edges'] as const) { + const arr = region!.properties?.[key]; + expect(arr, `${label}.${key}`).toBeDefined(); + expect(arr!.type, `${label}.${key}.type`).toBe('array'); + // The load-bearing absence: no element schema, so the designer treats the + // sub-graph as opaque instead of forms-for-every-node. + expect(arr!.items, `${label}.${key} must stay opaque (no items)`).toBeUndefined(); + } + } + + it('loop: template collection, bounded iterations, opaque body', () => { + const schema = schemaOf(engine2, 'loop'); + expect(schema.properties?.collection?.xExpression).toBe('template'); + expect(schema.properties?.maxIterations?.type).toBe('integer'); + expect(schema.properties?.maxIterations?.minimum).toBe(1); + expect(schema.properties?.maxIterations?.maximum).toBe(LOOP_MAX_ITERATIONS_CEILING); + expectOpaqueRegion(schema.properties?.body, 'loop.body'); + }); + + it('loop: the form does NOT carry the Zod-only string constraints', () => { + // `LoopConfigSchema.collection` is `z.string().min(1)` and + // `iteratorVariable` is `.default('item')`. Generating the form from that + // Zod would publish `minLength: 1` / `default: 'item'`, which this form has + // never advertised. Pinned so such a change is a decision, not a side effect. + const schema = schemaOf(engine2, 'loop'); + expect(schema.properties?.collection?.minLength).toBeUndefined(); + expect(schema.properties?.iteratorVariable?.minLength).toBeUndefined(); + expect(schema.properties?.iteratorVariable?.default).toBeUndefined(); + }); + + it('parallel: at least two branches, each an opaque region', () => { + const schema = schemaOf(engine2, 'parallel'); + expect(schema.properties?.branches?.type).toBe('array'); + expect(schema.properties?.branches?.minItems).toBe(2); + expect(schema.properties?.branches?.items?.properties?.name?.type).toBe('string'); + expectOpaqueRegion(schema.properties?.branches?.items, 'parallel.branches[]'); + }); + + it('try_catch: both regions opaque, bounded retry policy', () => { + const schema = schemaOf(engine2, 'try_catch'); + expectOpaqueRegion(schema.properties?.try, 'try_catch.try'); + expectOpaqueRegion(schema.properties?.catch, 'try_catch.catch'); + expect(schema.properties?.errorVariable?.type).toBe('string'); + const retry = schema.properties?.retry?.properties; + expect(retry?.maxRetries?.type).toBe('integer'); + expect(retry?.maxRetries?.minimum).toBe(0); + expect(retry?.maxRetries?.maximum).toBe(10); + expect(retry?.backoffMultiplier?.minimum).toBe(1); + expect(retry?.jitter?.type).toBe('boolean'); + }); + }); + it('decision / script stay deliberately schemaless (no partial forms)', () => { // A node with no configSchema renders identically online and offline (the // hardcoded fallback), so there is no divergence — and publishing a partial