From 9c0ce8bc98953e315181832778c08dec2cb9c2ed Mon Sep 17 00:00:00 2001 From: Claude Date: Thu, 30 Jul 2026 08:36:49 +0000 Subject: [PATCH] test(automation): reconcile the control-flow designer forms against their Zod (#4045) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `loop` / `parallel` / `try_catch` each carry two descriptions of the same config — a hand-written `configSchema` on the descriptor (drives the Studio form) and a Zod schema in `control-flow.zod.ts` (validates the value) — and nothing compared them. #4064 pinned the shape of the first; this pins the relationship, so neither side can gain or lose a key without the other noticing. Asserts both directions. A key the Zod accepts but the form omits is #3528's failure inverted: a real config key with no way to author it in Studio, discoverable only by reading the executor. A key the form offers but the Zod rejects means the author fills in a field whose value is then dropped. The two sources are deliberately NOT merged, and that is now measured rather than assumed. Generating the form from the Zod emits 9–17× more schema at +9 levels of depth (loop 597 → 5,537 chars / 25 → 201 keys; parallel 294 → 5,112; try_catch 737 → 10,739) with NO $defs and NO $ref — FlowNodeSchema is inlined in full at every region key, so a loop body would reach the designer as the entire node/edge definition instead of the opaque array a canvas-edited sub-graph needs. A projection pruning ~90% back off leaves three things to maintain instead of two, so "one source" would be a fiction. This corrects the premise #4045 opened with. Since the divergence is legitimate, what is enforced is that it stays DECLARED: a `DELIBERATELY_SHALLOW` ledger names each shallow key with a reason, and every entry must name a key both sides still declare — so it cannot rot into a reference to something renamed away. Compares key sets, not generated shapes, on purpose: generating needs `z.toJSONSchema` and `service-automation` does not depend on `zod` (only spec does). Adding that edge to the package graph to run a test is not worth it when `schema.shape` gives the key set without it, and the key set is where the drift that matters lives. The file says so, so the next reader does not think it was forgotten. Verified to bite in all three directions, not merely to pass: - a Zod key the form omits → `loop: accepted by the Zod but absent from the designer form: expected [ 'brandNewRegionKey' ] to deeply equal []` - a form key the Zod rejects → same test red on the inverse set - a ledger entry pointing at a renamed key → `loop.bodyRenamedAway: not on the form any more` Test plan: service-automation 457 passed (5 new), ESLint clean, and spec's api-surface / authorable-surface / docs gates still pass (the negative controls rebuilt spec, so the generated artifacts were checked back to clean). Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_01QoA8AV99Ss1RRkLLAYmDzq --- .changeset/control-flow-form-zod-ledger.md | 32 ++++ .../control-flow-form-zod-ledger.test.ts | 160 ++++++++++++++++++ 2 files changed, 192 insertions(+) create mode 100644 .changeset/control-flow-form-zod-ledger.md create mode 100644 packages/services/service-automation/src/builtin/control-flow-form-zod-ledger.test.ts diff --git a/.changeset/control-flow-form-zod-ledger.md b/.changeset/control-flow-form-zod-ledger.md new file mode 100644 index 0000000000..89efc01bf4 --- /dev/null +++ b/.changeset/control-flow-form-zod-ledger.md @@ -0,0 +1,32 @@ +--- +--- + +Tests only — no package changes, nothing to release. + +Reconciles the designer form against the Zod for the region-bearing flow nodes +(#4045). `loop` / `parallel` / `try_catch` each carry two descriptions of the same +config — a hand-written `configSchema` on the descriptor and a Zod schema in +`control-flow.zod.ts` — and nothing compared them. #4064 pinned the shape of the +first; this pins the relationship, so neither side can gain or lose a key without +the other noticing. + +The two are **not** merged into one source, because measurement says that does not +work here: generating from the Zod emits 9–17× more schema at +9 levels of depth +(`loop` 597 → 5,537 chars, `parallel` 294 → 5,112, `try_catch` 737 → 10,739), with +no `$defs`/`$ref` — `FlowNodeSchema` is inlined at every region key, so a loop body +would arrive as the whole node/edge definition instead of the opaque array the +designer needs for a canvas-edited sub-graph. A projection pruning ~90% back off +would leave three things to maintain instead of two. + +So the divergence is legitimate, and what is enforced is that it stays declared: a +`DELIBERATELY_SHALLOW` ledger names each shallow key with a reason, and every entry +must name a key both sides still declare, so it cannot rot. + +Compares key sets rather than generated shapes on purpose — generating needs +`z.toJSONSchema`, and `service-automation` does not depend on `zod`. The Zod's key +set is reachable via `schema.shape` without that dependency, and the key set is +where the drift that matters shows up. + +Verified to bite in all three directions: a Zod key the form does not offer, a form +key the Zod rejects, and a ledger entry pointing at a renamed key each turn the +suite red with the offending name in the message. diff --git a/packages/services/service-automation/src/builtin/control-flow-form-zod-ledger.test.ts b/packages/services/service-automation/src/builtin/control-flow-form-zod-ledger.test.ts new file mode 100644 index 0000000000..84ac0125ec --- /dev/null +++ b/packages/services/service-automation/src/builtin/control-flow-form-zod-ledger.test.ts @@ -0,0 +1,160 @@ +// Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license. + +/** + * **Designer form ↔ Zod reconciliation for the region-bearing nodes** (#4045). + * + * `loop` / `parallel` / `try_catch` each carry TWO descriptions of the same config: + * a hand-written `configSchema` on the descriptor (drives the Studio form) and a Zod + * schema in `control-flow.zod.ts` (validates the value). #4064 pinned the *shape* of + * the first; this pins the *relationship* between the two, so neither can gain or + * lose a key without the other noticing. + * + * ## Why they are not merged into one source + * + * The obvious de-duplication — publish `z.toJSONSchema(LoopConfigSchema)` and delete + * the literal — was measured on main `53fbf49` and is not viable for these three: + * + * | node | published form | generated from Zod | + * |---|---|---| + * | `loop` | 597 chars, depth 5, 25 keys | 5,537 chars, depth 14, 201 keys | + * | `parallel` | 294 chars, depth 6, 16 keys | 5,112 chars, depth 15, 189 keys | + * | `try_catch` | 737 chars, depth 5, 40 keys | 10,739 chars, depth 14, 397 keys | + * + * Generation succeeds — no recursion error — but emits **no `$defs` and no `$ref`**: + * `FlowNodeSchema` is inlined in full at every region key, so a loop body would + * arrive as the entire node/edge definition instead of the opaque `{ type: 'array' }` + * the designer needs for a sub-graph that is edited on the canvas. Making the + * generated output usable would mean a projection pruning ~90% of it back off, at + * which point "one source" is a fiction: you would maintain the Zod, the projection + * rules, and a check that the projection still does the right thing. + * + * So the two artifacts have different jobs and legitimately differ. What is worth + * enforcing is not that the difference disappears but that it stays **declared** — + * the #3569 / #4040 ledger pattern. + * + * ## Why this file compares KEY SETS and not generated shapes + * + * Deliberate, not an omission. Generating requires `z.toJSONSchema`, and + * `service-automation` does not depend on `zod` — only `@objectstack/spec` does. + * Adding that dependency to run a test would put a real edge in the package graph for + * no runtime need. The Zod's own key set is reachable without it (`schema.shape`), + * and the key set is where the drift that matters shows up: a key on one side and not + * the other. The depth divergence is measured once in the table above and its form + * half is pinned by `config-schemas.test.ts`. + */ + +import { describe, it, expect } from 'vitest'; +import { LoopConfigSchema, ParallelConfigSchema, TryCatchConfigSchema } from '@objectstack/spec/automation'; +import { AutomationEngine } from '../engine.js'; +import { registerLoopNode } from './loop-node.js'; +import { registerParallelNode } from './parallel-node.js'; +import { registerTryCatchNode } from './try-catch-node.js'; + +/** + * Config keys whose form shape is deliberately shallower than the Zod, with the + * reason. Checked both ways below — an entry must name a key both sides really + * declare, so it cannot rot into a reference to something that no longer exists. + */ +const DELIBERATELY_SHALLOW: ReadonlyArray<{ nodeType: string; key: string; why: string }> = [ + { + nodeType: 'loop', key: 'body', + why: 'FlowRegionSchema — the body sub-graph is edited on the canvas, so the form publishes an opaque array', + }, + { + nodeType: 'parallel', key: 'branches', + why: 'each branch carries a region; the form publishes name + opaque node/edge arrays', + }, + { + nodeType: 'try_catch', key: 'try', + why: 'protected region — opaque for the same reason as loop.body', + }, + { + nodeType: 'try_catch', key: 'catch', + why: 'handler region — opaque for the same reason as loop.body', + }, +]; + +const NODES: ReadonlyArray<{ nodeType: string; zod: unknown }> = [ + { nodeType: 'loop', zod: LoopConfigSchema }, + { nodeType: 'parallel', zod: ParallelConfigSchema }, + { nodeType: 'try_catch', zod: TryCatchConfigSchema }, +]; + +function engineWithControlFlow() { + const logger: any = { + info() {}, warn() {}, error() {}, debug() {}, + child() { return logger; }, + }; + const ctx: any = { logger, getService() { throw new Error('none'); } }; + const engine = new AutomationEngine(logger); + registerLoopNode(engine, ctx); + registerParallelNode(engine, ctx); + registerTryCatchNode(engine, ctx); + return engine; +} + +const engine = engineWithControlFlow(); + +/** Top-level keys the designer form offers for a node type. */ +function formKeys(nodeType: string): string[] { + const schema = engine.getActionDescriptor(nodeType)?.configSchema as + | { properties?: Record } + | undefined; + expect(schema, `${nodeType} should publish a configSchema`).toBeDefined(); + return Object.keys(schema!.properties ?? {}).sort(); +} + +/** + * Top-level keys the Zod object accepts, read straight off `.shape` — no `zod` + * import, so this stays inside the package's existing dependency graph. + */ +function zodKeys(schema: unknown): string[] { + const shape = (schema as { shape?: Record }).shape; + expect(shape, 'the Zod schema should expose a .shape').toBeDefined(); + return Object.keys(shape ?? {}).sort(); +} + +describe('control-flow form ↔ Zod reconciliation (#4045)', () => { + it.each(NODES)('$nodeType: the form offers exactly the keys the Zod accepts', ({ nodeType, zod }) => { + const form = formKeys(nodeType); + const zodded = zodKeys(zod); + + // Accepted by Zod, absent from the form ⇒ an author cannot set it in Studio. + // This is #3528's failure in the other direction: a real config key with no way + // to author it, discoverable only by reading the executor. + expect( + zodded.filter((k) => !form.includes(k)), + `${nodeType}: accepted by the Zod but absent from the designer form`, + ).toEqual([]); + + // Offered by the form, not accepted by the Zod ⇒ the author fills in a field + // whose value the parse then rejects or drops. + expect( + form.filter((k) => !zodded.includes(k)), + `${nodeType}: offered by the designer form but not accepted by the Zod`, + ).toEqual([]); + }); + + it('every ledger entry names a key both sides actually declare', () => { + // Stops the ledger rotting into references to keys that were renamed or removed — + // the same bidirectionality the #4040 expression ledger has. + for (const { nodeType, key } of DELIBERATELY_SHALLOW) { + const node = NODES.find((n) => n.nodeType === nodeType); + expect(node, `ledger references unknown node type '${nodeType}'`).toBeDefined(); + expect(formKeys(nodeType), `${nodeType}.${key}: not on the form any more`).toContain(key); + expect(zodKeys(node!.zod), `${nodeType}.${key}: not in the Zod any more`).toContain(key); + } + }); + + it('the ledger is not vacuous and every entry carries a reason', () => { + // The failure mode a ledger test must not have: if the entries stopped resolving, + // the assertions above would pass over an empty set. + expect(DELIBERATELY_SHALLOW.length).toBeGreaterThan(0); + for (const entry of DELIBERATELY_SHALLOW) { + expect( + entry.why.length, + `${entry.nodeType}.${entry.key} needs a reason a reader can act on`, + ).toBeGreaterThan(20); + } + }); +});