From 2f7c0a570838c4b2947806f1406a60f7b5ad7ed2 Mon Sep 17 00:00:00 2001 From: Claude Date: Mon, 28 Sep 2026 04:22:21 +0000 Subject: [PATCH 1/8] wip(spec,lint): one judge for the node config an executor requires, at the three build doors Claude-Session: https://claude.ai/code/session_01QcAS3qiYYZNezaxZxaUdMV Co-Authored-By: Claude --- packages/lint/src/validate-expressions.ts | 22 +- .../automation/flow-node-expression-paths.ts | 388 +++++++++++++++++- packages/spec/src/automation/flow.zod.ts | 44 +- 3 files changed, 445 insertions(+), 9 deletions(-) diff --git a/packages/lint/src/validate-expressions.ts b/packages/lint/src/validate-expressions.ts index 716388d9629..e1c22327f7d 100644 --- a/packages/lint/src/validate-expressions.ts +++ b/packages/lint/src/validate-expressions.ts @@ -87,6 +87,7 @@ import { } from '@objectstack/formula'; import { collectFlowGraphs, + flowNodeConfigRefusals, predicateSlotRefusal, resolveFlowNodeExpressions, resolveFlowNodeValueSlots, @@ -1620,6 +1621,22 @@ export function validateStackExpressions(stack: AnyRec): ExprIssue[] { // `loop.collection`). The ledger records them regardless, so the // reconciliation ratchet still sees the marker. const nodeType = typeof node.type === 'string' ? node.type : ''; + // [#20316] What the node's executor needs its `config` to carry — a key + // its contract requires, left out, and a `decision` branch list it + // cannot read. The spec's one judge, the same call `FlowSchema.parse` + // makes (and `registerFlow` meets through that parse), so a stack + // handed to `validateStackExpressions` without a parse in front of it + // is held to the same bar. `error`: the flow would register and then + // refuse — or, for a branch with no label, misroute — every run. + const configRefusals = flowNodeConfigRefusals(nodeType, node.config); + for (const refusal of configRefusals) { + issues.push({ + where: `${at} · node '${node.id}' (${nodeType}) config.${refusal.path}`, + message: refusal.message, + source: refusal.source, + severity: 'error', + }); + } for (const found of resolveFlowNodeExpressions(nodeType, cfg)) { const slotWhere = `${at} · node '${node.id}' (${nodeType}) ${found.entry.label} at config.${found.path}`; // [#15137] `value` slots are checkable too, by their own rule — see @@ -1727,7 +1744,10 @@ export function validateStackExpressions(stack: AnyRec): ExprIssue[] { + 'sources; apply them by hand.', source: JSON.stringify({ id: node.id, type: node.type, config: cfg }), }); - } else if (!fn) { + } else if (!fn && !configRefusals.some((r) => r.path === 'function')) { + // [#20316] An ABSENT `function` is the contract judge's finding + // above (one finding, not two); this arm keeps the shape that judge + // leaves alone — a `function` that is present and blank. issues.push({ where: `${at} · node '${node.id}' (script) callable`, message: diff --git a/packages/spec/src/automation/flow-node-expression-paths.ts b/packages/spec/src/automation/flow-node-expression-paths.ts index c9369640daf..28a395eae5f 100644 --- a/packages/spec/src/automation/flow-node-expression-paths.ts +++ b/packages/spec/src/automation/flow-node-expression-paths.ts @@ -75,6 +75,21 @@ // The repo's one notion of "blank" (`source.trim()`), shared with the evaluated // slots — never a second hand-written one here. import { NON_BLANK_STRING } from '../shared/refinement-projection'; +import { FLOW_REGION_SLOTS_BY_TYPE } from './region-slots'; +// The executor contracts (#20316). Read only inside +// `getBuiltinNodeConfigContracts`, never at module load: two of these modules +// import this one, so the bindings are live references resolved on first use. +import { LoopConfigSchema, ParallelConfigSchema, TryCatchConfigSchema } from './control-flow.zod'; +import { + CreateRecordConfigSchema, + DeleteRecordConfigSchema, + GetRecordConfigSchema, + MapConfigSchema, + ScreenConfigSchema, + UpdateRecordConfigSchema, +} from './builtin-node-config.zod'; +import { HttpConfigSchema, NotifyConfigSchema } from './io-node-config.zod'; +import { ScriptConfigSchema, SubflowConfigSchema } from './schemaless-node-config.zod'; /** * The dialect a declared expression slot takes — and therefore what, if @@ -168,8 +183,10 @@ export interface FlowNodeExpressionPath { * `service-automation`), in both directions over the `predicate` role, so * this flag cannot claim a requirement the contract does not make, nor miss * one it does. Never set on another role: the channels require - * `loop.collection` / `map.collection` too, but no door refuses their - * absence — their executors parse their own config. + * `loop.collection` / `map.collection` too, and since #20316 all three doors + * refuse their absence — but through {@link flowNodeConfigRefusals}, which + * judges every key an executor contract requires, not through this flag, + * which only decides what the expression walk emits. */ readonly required?: true; } @@ -353,7 +370,10 @@ export function isExpressionEnvelopeShaped(value: unknown): value is { dialect: * emitted (as `undefined` or `null`, whichever was there) for the consumer * to refuse through {@link predicateSlotRefusal}, the same way as a blank. * Only the element's OWN slot is judged: an element that is not an object - * carries no slot, and the walk does not reach it. A **non-string** + * carries no slot, and the walk does not reach it — an ARRAY element + * included, since #20316 (the walk used to read an array element as an + * object missing its slot); {@link flowNodeConfigRefusals} refuses such an + * element as what it is. A **non-string** * is emitted too (#15572), for the consumer to refuse through * {@link predicateSlotRefusal}: it used to be skipped as "a type violation * for the schema pass to report", and for a schemaless node type there is no @@ -483,9 +503,26 @@ export type StructuralConditionValueKind = | 'function'; /** - * Refusal code → the params its message interpolates, for this file's two - * refusal producers, {@link predicateSlotRefusal} and - * {@link structuralConditionRefusal}. The keys ARE the closed set. + * What a value sitting where a node's `config` wants another shape was, as a + * token — for {@link flowNodeConfigRefusals}. `null` is spelled out on the + * codes that can meet it; the message renders the token as a phrase (`a + * string`, `an array`, `an object`). + */ +export type NodeConfigValueKind = + | 'string' + | 'number' + | 'boolean' + | 'bigint' + | 'symbol' + | 'function' + | 'array' + | 'object'; + +/** + * Refusal code → the params its message interpolates, for this file's three + * refusal producers, {@link predicateSlotRefusal}, + * {@link structuralConditionRefusal} and {@link flowNodeConfigRefusals}. The + * keys ARE the closed set. * * A consumer that renders its own words — a localized designer — keys its * catalogue row to the `code` and fills it from the `params`; the English @@ -503,6 +540,23 @@ export interface FlowSlotRefusalParams { 'predicate-slot-not-text': { readonly found: PredicateSlotValueKind }; /** A structural condition holding neither text nor an envelope carrying a string `source`. */ 'structural-condition-shape': { readonly found: StructuralConditionValueKind }; + /** A `decision` node's `conditions` present, not `null`, and not an array. */ + 'decision-conditions-not-array': { readonly found: Exclude }; + /** An element of a `decision` node's `conditions` that is not an object (`null` and arrays included). */ + 'decision-branch-not-object': { readonly index: number; readonly found: Exclude | 'null' }; + /** A `decision` branch whose `label` is absent, `null`, blank after trimming, or not a string. */ + 'decision-branch-label-missing': { + readonly index: number; + readonly found: 'absent' | 'null' | 'blank' | Exclude; + }; + /** A key the node's executor contract requires, absent from the node's `config`. */ + 'node-config-key-missing': { readonly nodeType: string; readonly key: string }; + /** + * A key the node's executor contract requires IN THIS CONFIGURATION — by a + * rule of the contract's own, whose message is the refusal's — absent from + * the node's `config`. + */ + 'node-config-key-required-by-rule': { readonly nodeType: string; readonly key: string }; } /** Every refusal code this file's producers emit. */ @@ -514,6 +568,14 @@ export type PredicateSlotRefusalCode = 'predicate-slot-missing' | 'predicate-slo /** The codes {@link structuralConditionRefusal} emits. */ export type StructuralConditionRefusalCode = 'structural-condition-shape'; +/** The codes {@link flowNodeConfigRefusals} emits. */ +export type FlowNodeConfigRefusalCode = + | 'decision-conditions-not-array' + | 'decision-branch-not-object' + | 'decision-branch-label-missing' + | 'node-config-key-missing' + | 'node-config-key-required-by-rule'; + /** One refusal's `code` and `params`, correlated: narrowing on `code` narrows `params`. */ type FlowSlotRefusalOf = { message: string; source: string } & { [Code in Codes]: { readonly code: Code; readonly params: FlowSlotRefusalParams[Code] }; @@ -533,6 +595,15 @@ export type PredicateSlotRefusal = FlowSlotRefusalOf; */ export type StructuralConditionRefusal = FlowSlotRefusalOf; +/** + * One reason a node's `config` is refused on SHAPE or PRESENCE: the English + * `message`, the `source` to attribute it to (always `''` — none of these + * holds CEL text), the `code` with its `params`, and the `path` inside + * `config` it is anchored at, in the ledger's spelling (`conditions[0].label`, + * `fields[1].options[0].value`, `collection`). + */ +export type FlowNodeConfigRefusal = FlowSlotRefusalOf & { readonly path: string }; + /** * Keyed by code so the compiler holds {@link FLOW_SLOT_REFUSAL_CODES} equal to * {@link FlowSlotRefusalParams}. @@ -542,6 +613,11 @@ const FLOW_SLOT_REFUSAL_CODE_TABLE = { 'predicate-slot-blank': true, 'predicate-slot-not-text': true, 'structural-condition-shape': true, + 'decision-conditions-not-array': true, + 'decision-branch-not-object': true, + 'decision-branch-label-missing': true, + 'node-config-key-missing': true, + 'node-config-key-required-by-rule': true, } as const satisfies Record; /** @@ -866,6 +942,300 @@ export function structuralConditionRefusal( }; } +// ─── Node config the executor requires (#20316) ───────────────────── + +/** + * The executor contract a builtin node's `config` is parsed against at run + * time — the SAME Zod schema its executor hands `parseNodeConfig` + * (`service-automation/builtin/parse-config.ts`) — and, where the executor + * parses only on one path, the condition it parses on. + * + * Structural, like `parseNodeConfig`'s own view of a contract: this module + * reads `safeParse` and nothing else. + */ +export interface BuiltinNodeConfigContract { + readonly schema: { + safeParse(value: unknown): { + success: boolean; + error?: { issues: ReadonlyArray<{ code: string; path: ReadonlyArray; message: string }> }; + }; + }; + /** + * The executor parses the config only when this holds. Absent: always. The + * one member is `loop`, whose legacy flat-graph form (no `body`) predates + * the ADR-0031 construct its contract describes and is deliberately not + * parsed (`loop-node.ts`), so `collection` is required only once a `body` + * is there. + */ + readonly parsedWhen?: (config: Readonly>) => boolean; +} + +let cachedBuiltinNodeConfigContracts: ReadonlyMap | undefined; + +/** + * Every builtin node type whose executor parses its `config` against a + * contract at run time, keyed by `node.type` (#20316). + * + * The declared half of a pair: `service-automation`'s ratchet + * (`node-config-contract-ledger.test.ts`) reads each executor's + * `parseNodeConfig(…)` call out of its source and holds this map equal to it + * in both directions — the type, the schema, and `loop`'s parse condition — + * so a new contract-parsing executor cannot go unjudged here, and an entry + * cannot outlive the parse it mirrors. + * + * Built on first use, never at module load: these schemas' modules import + * this one, and a map literal at top level would read them mid-cycle. + * + * NOT here, on purpose: `decision` (its executor parses nothing — its branch + * shape is judged by {@link flowNodeConfigRefusals}'s own arm), `assignment` + * (three read-compatible shapes, no single contract), and `wait` / + * `connector_action`, whose inputs are FlowNode SIBLING blocks + * (`waitEventConfig` / `connectorConfig`), not `config`. + */ +export function getBuiltinNodeConfigContracts(): ReadonlyMap { + if (cachedBuiltinNodeConfigContracts === undefined) { + cachedBuiltinNodeConfigContracts = new Map([ + ['get_record', { schema: GetRecordConfigSchema }], + ['create_record', { schema: CreateRecordConfigSchema }], + ['update_record', { schema: UpdateRecordConfigSchema }], + ['delete_record', { schema: DeleteRecordConfigSchema }], + ['notify', { schema: NotifyConfigSchema }], + ['http', { schema: HttpConfigSchema }], + ['screen', { schema: ScreenConfigSchema }], + ['script', { schema: ScriptConfigSchema }], + ['subflow', { schema: SubflowConfigSchema }], + ['map', { schema: MapConfigSchema }], + ['loop', { schema: LoopConfigSchema, parsedWhen: (config) => config.body != null }], + ['parallel', { schema: ParallelConfigSchema }], + ['try_catch', { schema: TryCatchConfigSchema }], + ]); + } + return cachedBuiltinNodeConfigContracts; +} + +/** A plain object (not an array, not `null`). */ +function isRecord(value: unknown): value is Record { + return typeof value === 'object' && value !== null && !Array.isArray(value); +} + +/** The kind of a value that is not the shape a node-config position wants. */ +function nodeConfigValueKind(value: unknown): NodeConfigValueKind { + if (Array.isArray(value)) return 'array'; + return typeof value as NodeConfigValueKind; +} + +/** `a string`, `an array`, `an object` — the phrase a kind token renders as. */ +function kindPhrase(kind: NodeConfigValueKind | 'null'): string { + if (kind === 'null') return '`null`'; + return kind === 'array' || kind === 'object' ? `an ${kind}` : `a ${kind}`; +} + +/** A Zod issue path → the ledger spelling (`['fields', 0, 'name']` → `fields[0].name`). */ +function ledgerPathOf(path: ReadonlyArray): string { + let out = ''; + for (const segment of path) { + if (typeof segment === 'number') out += `[${segment}]`; + else out += out ? `.${String(segment)}` : String(segment); + } + return out; +} + +/** + * Is the key an issue names ABSENT from the authored config — its parent + * reached, and the key itself not there (or `undefined`)? The one question + * the contract arm asks: a present value of the wrong type is a different + * finding, and not this judge's. + */ +function absentAt(config: Readonly>, path: ReadonlyArray): boolean { + let parent: unknown = config; + for (const segment of path.slice(0, -1)) { + if (parent === null || typeof parent !== 'object') return false; + parent = (parent as Record)[segment as PropertyKey]; + } + if (parent === null || typeof parent !== 'object') return false; + const last = path[path.length - 1] as PropertyKey; + return (parent as Record)[last] === undefined; +} + +/** + * Does an issue path descend INTO an ADR-0031 region (`body.nodes…`, + * `branches[0]…`, `try.edges…`)? Those are the region's own nodes and edges, + * judged where the walks reach them as a graph — never re-reported against + * the container that holds them. The slot itself absent (`try`, `branches`) + * is the container's, and is judged here. + */ +function insideRegion(nodeType: string, path: ReadonlyArray): boolean { + if (path.length < 2) return false; + return (FLOW_REGION_SLOTS_BY_TYPE.get(nodeType) ?? []).some((slot) => slot.key === path[0]); +} + +/** The refusal for a key a node's executor contract requires. */ +function nodeConfigKeyMissingMessage(nodeType: string, key: string): string { + return ( + `This \`${nodeType}\` node's config leaves out \`${key}\`, which the ${nodeType} contract requires. Its executor ` + + 'parses the config against that contract before it does anything else and refuses the node without it — so ' + + 'the flow registers, and then every run that reaches this node fails there; the config is metadata, and ' + + `re-running changes nothing. Write \`${key}\` on the node's \`config\`.` + ); +} + +/** + * Every reason a node's `config` is refused on SHAPE or PRESENCE — the ONE + * judge `FlowSchema.parse`, `AutomationEngine.registerFlow` (which parses + * first) and `objectstack validate` share (#20316), in this module beside + * {@link predicateSlotRefusal} because it closes the same gap: a node's + * `config` is an open `z.record`, so what its executor requires was checked + * by nobody until the run. + * + * Two arms. + * + * ## The executor contract — a key it requires, absent + * + * For a type in {@link getBuiltinNodeConfigContracts}, the config is parsed + * against the executor's own contract, on the executor's own condition + * (`config ?? {}`, as `parseNodeConfig` reads it; `loop` only with a `body`), + * and a failure is kept ONLY where the key it names is absent from what was + * authored. That keeps the judge to one question — "would the run refuse this + * node for a key it leaves out?" — and leaves every other contract finding + * (a present value of the wrong type, an undeclared key) where it lives + * today. Issues inside an ADR-0031 region are the region's own and skipped. + * + * - A key the contract simply requires → `node-config-key-missing`, whose + * message names the key and the node type. + * - A key a RULE of the contract requires in this configuration (a `notify` + * with no `template` needs `title`; a `lookup` screen field needs its + * `reference`) → `node-config-key-required-by-rule`, whose message is the + * contract's own. + * + * A key only the conversion layer spells canonically (`object` → + * `objectName`, `flow` → `flowName`, …) is judged AFTER the conversion at + * `registerFlow` and `objectstack validate`, which convert first; a direct + * `FlowSchema.parse` of a pre-conversion spelling meets the refusal, exactly + * as it meets every other tombstone. + * + * ## The decision branch shape + * + * `decision` is parsed by nothing at run time — its executor reads + * `conditions[]` raw — so its arm states what that read needs: + * + * - `conditions` present and not `null` is an array + * (`decision-conditions-not-array`) — the executor iterates it; + * - every element is an object (`decision-branch-not-object`) — the + * executor reads `label` and `expression` off it; + * - every branch's `label` is a non-blank string + * (`decision-branch-label-missing`) — the label is the branch: the matched + * branch reports it and traversal keeps only the out-edge carrying it, so + * a branch without one selects nothing and EVERY out-edge is considered. + * + * The branch's `expression` is not judged here: it is a ledger `predicate` + * slot, refused absent / blank / non-text by {@link predicateSlotRefusal}. + * + * Every refusal carries `source: ''`: none of these values is CEL text. + */ +export function flowNodeConfigRefusals(nodeType: string, config: unknown): FlowNodeConfigRefusal[] { + const out: FlowNodeConfigRefusal[] = []; + if (nodeType === 'decision') { + decisionShapeRefusals(config, out); + return out; + } + const contract = getBuiltinNodeConfigContracts().get(nodeType); + if (!contract) return out; + const authored = config ?? {}; + if (!isRecord(authored)) return out; + if (contract.parsedWhen && !contract.parsedWhen(authored)) return out; + const result = contract.schema.safeParse(authored); + if (result.success) return out; + const seen = new Set(); + for (const issue of result.error?.issues ?? []) { + if (issue.path.length === 0) continue; + if (insideRegion(nodeType, issue.path)) continue; + if (!absentAt(authored, issue.path)) continue; + const key = ledgerPathOf(issue.path); + if (seen.has(key)) continue; + seen.add(key); + out.push( + issue.code === 'custom' + ? { code: 'node-config-key-required-by-rule', params: { nodeType, key }, message: issue.message, source: '', path: key } + : { + code: 'node-config-key-missing', + params: { nodeType, key }, + message: nodeConfigKeyMissingMessage(nodeType, key), + source: '', + path: key, + }, + ); + } + return out; +} + +/** The `decision` arm of {@link flowNodeConfigRefusals}. */ +function decisionShapeRefusals(config: unknown, out: FlowNodeConfigRefusal[]): void { + if (!isRecord(config)) return; + const conditions = config.conditions; + // Absent or `null` declares no branch: the executor reads `?? []` and the + // node routes by its out-edges, which is legal. + if (conditions == null) return; + if (!Array.isArray(conditions)) { + const found = nodeConfigValueKind(conditions) as Exclude; + out.push({ + code: 'decision-conditions-not-array', + params: { found }, + message: + `A decision's \`conditions\` is its ordered branch list — an array of \`{ label, expression }\` — and this ` + + `one is ${kindPhrase(found)}. The decision executor iterates it, so a run that reaches the node fails ` + + 'there (a string is iterated character by character, each character a branch with no `expression`). ' + + 'Write the branches as an array, or delete `conditions` and route by the out-edges\' own `condition`s.', + source: '', + path: 'conditions', + }); + return; + } + conditions.forEach((branch: unknown, index: number) => { + const path = `conditions[${index}]`; + if (!isRecord(branch)) { + const found = branch === null ? 'null' : (nodeConfigValueKind(branch) as Exclude); + out.push({ + code: 'decision-branch-not-object', + params: { index, found }, + message: + `A decision branch is an object — \`{ label, expression }\` — and \`${path}\` is ${kindPhrase(found)}. ` + + 'The decision executor reads `label` and `expression` off every branch it reaches, so this one has ' + + 'neither and a run that reaches it fails at the node. Write it as `{ label: \'approved\', expression: ' + + '\'record.amount > 1000\' }` — the label of the out-edge it routes to, and a bare CEL predicate; a ' + + 'predicate written as a bare string belongs under `expression`.', + source: '', + path, + }); + return; + } + const label = branch.label; + if (typeof label === 'string' && NON_BLANK_STRING(label)) return; + const found: FlowSlotRefusalParams['decision-branch-label-missing']['found'] = + label === undefined ? 'absent' + : label === null ? 'null' + : typeof label === 'string' ? 'blank' + : (nodeConfigValueKind(label) as Exclude); + const phrase = + found === 'absent' ? 'nothing — the key is absent' + : found === 'blank' ? 'a string that is blank after trimming' + : kindPhrase(found); + out.push({ + code: 'decision-branch-label-missing', + params: { index, found }, + message: + 'A decision branch routes by its `label`: the first branch whose `expression` holds is taken, and the run ' + + `continues down the out-edge carrying that label. \`${path}.label\` holds ${phrase}, which names no ` + + 'out-edge — so when this branch matches, the node reports no branch it can route, and traversal ' + + 'considers EVERY out-edge instead, as if the decision declared no branches: an unconditional labelled ' + + 'out-edge and the default out-edge both run. Write the label of the out-edge this branch should take ' + + '(`label: \'approved\'` for the out-edge labelled `approved`). To branch on the out-edges instead, ' + + 'delete `conditions` and put each predicate on its edge\'s `condition`.', + source: '', + path: `${path}.label`, + }); + }); +} + /** * Descend `segments` through `node`, expanding a `key[]` segment over every * element of that array and a `*` segment over every own key of that object, @@ -880,6 +1250,12 @@ function walk( if (node == null || typeof node !== 'object') return; const [head, ...rest] = segments; if (head === undefined) return; + // A named key is read off an OBJECT (#20316). An array element where the + // path expects an object carries no slot at all — it is a malformed element, + // refused as such by `flowNodeConfigRefusals` (`decision-branch-not-object`) + // — so it must not be read as an object whose required slot is absent and + // refused a second time, for the wrong reason. + if (Array.isArray(node) && head !== '*' && !head.endsWith('[]')) return; if (head === '*') { // Every own key of a plain object (#14149). An array here is not "a map diff --git a/packages/spec/src/automation/flow.zod.ts b/packages/spec/src/automation/flow.zod.ts index 71b1ee94ccc..982c6cbda4a 100644 --- a/packages/spec/src/automation/flow.zod.ts +++ b/packages/spec/src/automation/flow.zod.ts @@ -23,7 +23,7 @@ import { retiredKey } from '../shared/retired-key'; import { retryPolicyShape } from '../shared/retry-policy.zod'; import { strictObject } from '../shared/strict-object'; import { collectFlowGraphs, parseFlowNodeRegions } from './control-flow.zod'; -import { predicateSlotRefusal, resolveFlowNodeExpressions } from './flow-node-expression-paths'; +import { flowNodeConfigRefusals, predicateSlotRefusal, resolveFlowNodeExpressions } from './flow-node-expression-paths'; import { EndConfigSchema } from './builtin-node-config.zod'; import { APPROVAL_NODE_TYPE, APPROVAL_REVISE_NODE_TYPE } from './approval.zod'; export const FlowNodeAction = z.enum([ @@ -161,7 +161,11 @@ export const FLOW_PAUSE_CAPABLE_NODE_TYPES: readonly string[] = [ * executor's to close. One VALUE rule reaches into an open `config` at the * flow level without closing its key set (#17493): a blank string in a slot * the expression ledger declares with the `predicate` role is refused by the - * `FlowSchema` superRefine — see the block there for its scope. + * `FlowSchema` superRefine — see the block there for its scope. A PRESENCE + * rule reaches in beside it, still without closing the key set (#20316): a + * key the node's executor contract requires, left out, and a `decision` + * branch list the executor cannot read — `flowNodeConfigRefusals`, in the + * same superRefine. */ /** @@ -1351,6 +1355,42 @@ export const FlowSchema = lazySchema(() => strictObject( }); } + // What a node's executor needs its `config` to carry (#20316) — the ONE + // judge `flowNodeConfigRefusals`, shared with `objectstack validate`'s + // expression pass, and met at `AutomationEngine.registerFlow` through this + // parse (it parses first). Two arms, both stated where they are judged: + // + // - a key the node's EXECUTOR CONTRACT requires, left out — the contract + // being the very schema the executor's `parseNodeConfig` call refuses the + // node against at run time, so a flow carrying one used to register and + // then fail every run that reached the node (`loop` with a `body` and no + // `collection`, `map` with no `collection`, a CRUD node with no + // `objectName`, …). Only ABSENCE is judged: a present value of the wrong + // type, or an undeclared key, stays where it is judged today; + // - a `decision` branch list the executor cannot read — `conditions` not an + // array, a branch that is not an object, and a branch whose `label` is + // absent, blank or not text. The last one never failed a run at all: the + // matched branch reported no label, and traversal took EVERY out-edge. + // + // A PRESENCE rule, never a key-set closure: the node `config` stays the open + // record the header of this module describes. Walked with + // `collectFlowGraphs`, so a node inside an ADR-0031 region body is judged at + // the path the author wrote; a container's own judgement skips its regions' + // insides, which this same walk reaches as graphs of their own. + for (const graph of collectFlowGraphs(flow)) { + graph.nodes.forEach((node, index) => { + const type: unknown = (node as { type?: unknown } | null)?.type; + if (typeof type !== 'string') return; + for (const refusal of flowNodeConfigRefusals(type, (node as { config?: unknown }).config)) { + ctx.addIssue({ + code: 'custom', + path: [...graph.path, 'nodes', index, 'config', ...ledgerPathSegments(refusal.path)], + message: refusal.message, + }); + } + }); + } + // Edges (#14964): every reader of `edges[].id` assumes the ids are unique — // a designer, a BPMN export, a flow diff, any traversal that dedupes by id — // while nothing enforced it: two edges carrying one id parsed, shipped From 337000464946d77677d0a9405e39010d26e958b4 Mon Sep 17 00:00:00 2001 From: Claude Date: Mon, 28 Sep 2026 04:45:42 +0000 Subject: [PATCH 2/8] wip: pins at the three doors, the executor-contract ledger ratchet, the ADR-0087 D3 entry Claude-Session: https://claude.ai/code/session_01QcAS3qiYYZNezaxZxaUdMV Co-Authored-By: Claude --- .../lint/src/validate-expressions.test.ts | 65 +++++ .../builtin/config-expression-ledger.test.ts | 29 +- .../node-config-contract-ledger.test.ts | 100 +++++++ .../src/node-config-required-keys.test.ts | 213 +++++++++++++++ .../flow-node-config-required.test.ts | 254 ++++++++++++++++++ .../automation/flow-node-expression-paths.ts | 2 +- .../flow-slot-refusal-codes.test.ts | 195 +++++++++++++- ....flow-node-config-required-keys-refused.ts | 74 +++++ packages/spec/src/migrations/registry.ts | 70 +++++ 9 files changed, 993 insertions(+), 9 deletions(-) create mode 100644 packages/services/service-automation/src/builtin/node-config-contract-ledger.test.ts create mode 100644 packages/services/service-automation/src/node-config-required-keys.test.ts create mode 100644 packages/spec/src/automation/flow-node-config-required.test.ts create mode 100644 packages/spec/src/migrations/entries/semantic/18.flow-node-config-required-keys-refused.ts diff --git a/packages/lint/src/validate-expressions.test.ts b/packages/lint/src/validate-expressions.test.ts index bb19b412dca..9186919d1a8 100644 --- a/packages/lint/src/validate-expressions.test.ts +++ b/packages/lint/src/validate-expressions.test.ts @@ -15,6 +15,7 @@ import { ASSIGNMENT_VALUE_ENVELOPE_REFUSAL, PREDICATE_SLOT_STRING_REFUSAL, STRUCTURAL_CONDITION_SHAPE_REFUSAL, + flowNodeConfigRefusals, predicateSlotRefusal, } from '@objectstack/spec/automation'; @@ -4468,6 +4469,70 @@ describe('a decision branch with no `expression` (#19961)', () => { }); }); +/** + * [#20316] What a node's executor needs its `config` to carry, at the THIRD + * door: `objectstack validate`'s expression pass. + * + * A key the node's executor contract requires, left out, and a `decision` + * branch list the executor cannot read, reported NOTHING here — the flow then + * registered and every run that reached the node failed there, or (a branch + * with no `label`) ran green down every out-edge. This pass now refuses them + * through `flowNodeConfigRefusals` — the same call, the same message, as + * `FlowSchema.parse` and `registerFlow`. + * + * ⚠️ Through the CLI, `objectstack validate` meets these shapes first at its + * schema step (`FlowSchema.parse` refuses them there, with the same message). + * This pass answers for a stack handed to `validateStackExpressions` directly, + * and it is what these pins drive. + */ +describe('node config an executor requires (#20316)', () => { + const stackWith = (node: Record) => ({ + flows: [{ name: 'config_flow', nodes: [{ id: 'start', type: 'start' }, { id: 'n', ...node }], edges: [] }], + }); + const errorsOf = (stack: unknown) => + validateStackExpressions(stack as never).filter((i) => (i.severity ?? 'error') === 'error'); + + it.each([ + ['loop', { collection: '{rows}', body: { nodes: [{ id: 'b', type: 'assignment' }], edges: [] } }, 'collection'], + ['map', { collection: '{rows}', flowName: 'child_flow' }, 'collection'], + ['get_record', { objectName: 'account' }, 'objectName'], + ['http', { url: 'https://example.com/hook' }, 'url'], + ['script', { function: 'recalc_totals' }, 'function'], + ] as Array<[string, Record, string]>)('%s without `%s` is refused; with it, nothing is', (type, whole, key) => { + expect(errorsOf(stackWith({ type, config: whole }))).toHaveLength(0); + const authored = { ...whole }; + delete authored[key]; + const found = errorsOf(stackWith({ type, config: authored })); + expect(found.map((i) => [i.where, i.message, i.source])).toEqual([ + [`flow 'config_flow' · node 'n' (${type}) config.${key}`, flowNodeConfigRefusals(type, authored)[0].message, ''], + ]); + }); + + it('a decision branch with no `label` is refused at the label; its labelled twin is not', () => { + const found = errorsOf(stackWith({ type: 'decision', config: { conditions: [{ expression: 'true' }] } })); + expect(found.map((i) => [i.where, i.message])).toEqual([[ + "flow 'config_flow' · node 'n' (decision) config.conditions[0].label", + flowNodeConfigRefusals('decision', { conditions: [{ expression: 'true' }] })[0].message, + ]]); + expect(errorsOf(stackWith({ type: 'decision', config: { conditions: [{ label: 'y', expression: 'true' }] } }))).toHaveLength(0); + }); + + it('a decision branch that is a bare string is refused once, as a branch', () => { + const found = errorsOf(stackWith({ type: 'decision', config: { conditions: ['true'] } })); + expect(found.map((i) => i.where)).toEqual(["flow 'config_flow' · node 'n' (decision) config.conditions[0]"]); + }); + + it('a `script` with no `function` is ONE finding — the judge\'s, not also the callable check\'s', () => { + const found = errorsOf(stackWith({ type: 'script', config: {} })); + expect(found.map((i) => i.where)).toEqual(["flow 'config_flow' · node 'n' (script) config.function"]); + }); + + it('CONTROL — a `script` whose `function` is present but blank keeps the callable check\'s finding', () => { + const found = errorsOf(stackWith({ type: 'script', config: { function: ' ' } })); + expect(found.map((i) => i.where)).toEqual(["flow 'config_flow' · node 'n' (script) callable"]); + }); +}); + /** * [#20078] A field-level predicate that reads THROUGH a reference field is * refused at authoring, with the repair that is true for the root it reads. diff --git a/packages/services/service-automation/src/builtin/config-expression-ledger.test.ts b/packages/services/service-automation/src/builtin/config-expression-ledger.test.ts index fa8e9821f0b..abafeab1ad7 100644 --- a/packages/services/service-automation/src/builtin/config-expression-ledger.test.ts +++ b/packages/services/service-automation/src/builtin/config-expression-ledger.test.ts @@ -28,6 +28,7 @@ import { describe, it, expect } from 'vitest'; import { FLOW_NODE_EXPRESSION_PATHS, + flowNodeConfigRefusals, getSchemalessNodeConfigJsonSchemas, resolveFlowNodeExpressions, type FlowNodeExpressionRole, @@ -231,9 +232,11 @@ describe('configSchema ↔ expression-ledger reconciliation (#4027)', () => { * Reconciled over the `predicate` role, the one role the flag acts on: the * resolver emits an absent value only for a required PREDICATE slot. The * channels require two `flow-template` slots too (`loop.collection`, - * `map.collection`), and no door refuses their absence — their executors - * parse their own config — so the flag stays off there, and the second - * assertion pins that it is never set on another role. + * `map.collection`), so the flag stays off there, and the second assertion + * pins that it is never set on another role. Their absence is refused at the + * three doors all the same since #20316 — by `flowNodeConfigRefusals`, the + * judge of every key an executor contract requires — and the test after + * this one holds that cross-check. */ it('the ledger marks `required` exactly the predicate slots the declaring channel requires (#19961)', () => { const declared = declaredEverywhere().filter((d) => d.role === 'predicate'); @@ -247,6 +250,26 @@ describe('configSchema ↔ expression-ledger reconciliation (#4027)', () => { expect(declared.filter((d) => !d.required).map(key)).toEqual(['screen.fields[].visibleWhen (predicate)']); }); + /** + * [#20316] The cross-check the census rests on: every slot a declaring + * channel REQUIRES but the ledger's `required` flag leaves alone (the + * non-predicate ones — today `loop.collection` and `map.collection`) is + * refused ABSENT by the node-config judge, on a config that is otherwise + * whole. Before #20316 all three doors admitted both, and each run refused. + */ + it('every channel-required slot outside the predicate role is refused absent by flowNodeConfigRefusals (#20316)', () => { + const requiredElsewhere = declaredEverywhere().filter((d) => d.required && d.role !== 'predicate'); + expect(requiredElsewhere.map(key).sort()).toEqual(['loop.collection (flow-template)', 'map.collection (flow-template)']); + const WHOLE: Record> = { + loop: { body: { nodes: [{ id: 'b', type: 'assignment', label: 'B' }], edges: [] } }, + map: { flowName: 'child_flow' }, + }; + for (const slot of requiredElsewhere) { + const refusals = flowNodeConfigRefusals(slot.nodeType, WHOLE[slot.nodeType]); + expect(refusals.map((r) => [r.code, r.path]), key(slot)).toEqual([['node-config-key-missing', slot.path]]); + } + }); + it('decision.conditions[].expression is covered — the #4439 hole', () => { const decision = FLOW_NODE_EXPRESSION_PATHS.find( (e) => e.nodeType === 'decision' && e.path === 'conditions[].expression', diff --git a/packages/services/service-automation/src/builtin/node-config-contract-ledger.test.ts b/packages/services/service-automation/src/builtin/node-config-contract-ledger.test.ts new file mode 100644 index 00000000000..0cbfce5e990 --- /dev/null +++ b/packages/services/service-automation/src/builtin/node-config-contract-ledger.test.ts @@ -0,0 +1,100 @@ +// Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license. + +/** + * **executor `parseNodeConfig` ↔ spec contract map reconciliation** (#20316). + * + * The build doors refuse a key a node's executor contract requires, left out, + * through the spec's one judge `flowNodeConfigRefusals` — which judges each + * node against `getBuiltinNodeConfigContracts()`. That map is only right while + * it says what the executors actually do, so this test reads the executors: + * + * - every `parseNodeConfig('', node.id, , …)` call in this + * directory's executor sources, and the map, name the same node types, and + * each type's map entry holds the SAME schema object the executor parses + * against — a new contract-parsing builtin with no map entry fails here + * (its missing keys would otherwise pass every door again, the #20316 + * shape), and so does an entry whose parse is gone; + * - `loop` is the one executor that parses on a condition (its legacy + * flat-graph form, no `body`, is not parsed), and the map's `parsedWhen` + * mirrors that guard; + * - every builtin node type is classified: judged by a contract, or on the + * short, reasoned list of types whose executor parses no `config` contract. + */ + +import { readFileSync, readdirSync } from 'node:fs'; +import { dirname, join } from 'node:path'; +import { fileURLToPath } from 'node:url'; +import { describe, expect, it } from 'vitest'; +import * as automation from '@objectstack/spec/automation'; +import { getBuiltinNodeConfigContracts } from '@objectstack/spec/automation'; +import { AutomationEngine } from '../engine.js'; +import { installBuiltinNodes } from './index.js'; + +const HERE = dirname(fileURLToPath(import.meta.url)); + +/** `parseNodeConfig<…>('type', node.id, Schema, …)` — the one call shape every executor uses. */ +const PARSE_CALL = /parseNodeConfig<\w+>\(\s*'([a-z_]+)'\s*,\s*node\.id\s*,\s*(\w+)\s*,/g; + +function executorParses(): Map { + const out = new Map(); + for (const file of readdirSync(HERE)) { + if (!file.endsWith('.ts') || file.endsWith('.test.ts') || file === 'parse-config.ts') continue; + const source = readFileSync(join(HERE, file), 'utf8'); + for (const match of source.matchAll(PARSE_CALL)) { + expect(out.has(match[1]), `${match[1]} parses its config in two places`).toBe(false); + out.set(match[1], { schemaName: match[2], file }); + } + } + return out; +} + +/** + * Builtins whose executor parses NO `config` contract, each with its reason — + * the other half of the classification. A new builtin lands here only by an + * edit that states why. + */ +const NO_CONFIG_CONTRACT: Readonly> = { + decision: 'reads `conditions[]` raw; its branch shape is judged by flowNodeConfigRefusals\' own decision arm', + assignment: 'normalizes three read-compatible shapes; no single contract describes them', + wait: 'its input is the FlowNode sibling block `waitEventConfig`, required by FlowSchema itself', + connector_action: 'its input is the FlowNode sibling block `connectorConfig`, not `config`', +}; + +describe('executor parseNodeConfig ↔ getBuiltinNodeConfigContracts (#20316)', () => { + const parses = executorParses(); + const contracts = getBuiltinNodeConfigContracts(); + + it('the derivation is not vacuous', () => { + expect(parses.size).toBeGreaterThanOrEqual(13); + }); + + it('the map names exactly the node types an executor parses a config contract for', () => { + expect([...contracts.keys()].sort()).toEqual([...parses.keys()].sort()); + }); + + it('each map entry holds the very schema object its executor parses against', () => { + for (const [type, { schemaName, file }] of parses) { + const published = (automation as Record)[schemaName]; + expect(published, `${file}: ${schemaName} is not published by @objectstack/spec/automation`).toBeDefined(); + expect(contracts.get(type)?.schema, `${type}: the map's schema is not ${schemaName}`).toBe(published); + } + }); + + it('`loop` alone parses on a condition — its executor skips the legacy form with no `body`, and the map mirrors it', () => { + const loopSource = readFileSync(join(HERE, parses.get('loop')!.file), 'utf8'); + expect(loopSource).toContain('if (raw.body == null) {'); + const loop = contracts.get('loop')!; + expect(loop.parsedWhen?.({})).toBe(false); + expect(loop.parsedWhen?.({ body: null })).toBe(false); + expect(loop.parsedWhen?.({ body: { nodes: [] } })).toBe(true); + expect([...contracts].filter(([, c]) => c.parsedWhen).map(([t]) => t)).toEqual(['loop']); + }); + + it('every builtin node type is classified — judged by a contract, or on the reasoned no-contract list', () => { + const engine = new AutomationEngine({ info() {}, warn() {}, error() {}, debug() {} } as never); + installBuiltinNodes(engine, { logger: { info() {}, warn() {}, error() {}, debug() {} }, getService() { throw new Error('none'); } } as never); + const builtins = engine.getActionDescriptors().map((d) => d.type).sort(); + const classified = [...contracts.keys(), ...Object.keys(NO_CONFIG_CONTRACT)].sort(); + expect(builtins).toEqual(classified); + }); +}); diff --git a/packages/services/service-automation/src/node-config-required-keys.test.ts b/packages/services/service-automation/src/node-config-required-keys.test.ts new file mode 100644 index 00000000000..fe4a67444d7 --- /dev/null +++ b/packages/services/service-automation/src/node-config-required-keys.test.ts @@ -0,0 +1,213 @@ +// Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license. + +/** + * #20316 — what a node's executor needs its `config` to carry is refused at + * `AutomationEngine.registerFlow`, the second of the three doors. + * + * Measured before the change, on every contract-parsing builtin: a flow whose + * node left out a key the node's executor contract requires REGISTERED, and + * then every run that reached the node failed there — `parseNodeConfig` + * refusing it as a guard. And a `decision` branch with no `label` registered + * and ran GREEN, down every out-edge. + * + * ## Which gate answers, measured rather than assumed + * + * `registerFlow` parses first (`canonicalizeStoredFlow` → `FlowSchema.parse`), + * and the flow parse refuses these shapes itself, through the spec's one judge + * `flowNodeConfigRefusals` — so what this door hands back is the PARSE's Zod + * issue, `custom`, anchored at the key. The census below is the one + * `FlowSchema.parse` runs in `spec` (`flow-node-config-required.test.ts`), and + * each executor below is the REAL builtin, so a refused row is also shown to + * be one the run refused. + */ +import { describe, expect, it, vi } from 'vitest'; +import { flowNodeConfigRefusals } from '@objectstack/spec/automation'; + +import { AutomationEngine } from './engine.js'; +import { installBuiltinNodes } from './builtin/index.js'; + +const silentLogger = { info: vi.fn(), warn: vi.fn(), error: vi.fn(), debug: vi.fn(), child: () => silentLogger } as any; +const ctx = { logger: silentLogger, getService: () => { throw new Error('none'); } } as any; + +type Node = Record; +type Config = Record; + +function builtinEngine(visited?: string[]): AutomationEngine { + const engine = new AutomationEngine(silentLogger); + installBuiltinNodes(engine, ctx); + engine.registerNodeExecutor({ + type: 'mark', + async execute(node) { + visited?.push(node.id); + return { success: true }; + }, + }); + engine.sealNodeTypeVocabulary(); + return engine; +} + +function flowWith(name: string, ...middle: Node[]) { + const nodes: Node[] = [{ id: 'start', type: 'start', label: 'Start' }, ...middle]; + const edges = nodes.slice(1).map((n, i) => ({ id: `e${i}`, source: String(nodes[i].id), target: String(n.id) })); + return { name, label: name, type: 'autolaunched', nodes, edges }; +} + +/** What `registerFlow` threw, or `undefined` when it registered. */ +function refusalOf(engine: AutomationEngine, flow: { name: string }): { issues?: Array<{ code: string; path: unknown[]; message: string }> } | undefined { + try { + engine.registerFlow(flow.name, flow as never); + return undefined; + } catch (e) { + return e as never; + } +} + +function segmentsOf(path: string): (string | number)[] { + return path.split('.').flatMap((part) => { + const bracket = part.indexOf('['); + if (bracket === -1) return [part]; + return [part.slice(0, bracket), ...[...part.slice(bracket).matchAll(/\[(\d+)\]/g)].map((m) => Number(m[1]))]; + }); +} + +function without(config: Config, path: string): Config { + const copy = JSON.parse(JSON.stringify(config)) as Config; + const segments = segmentsOf(path); + let parent: any = copy; + for (const segment of segments.slice(0, -1)) parent = parent[segment]; + delete parent[segments[segments.length - 1]]; + return copy; +} + +const region = (tag: string) => ({ nodes: [{ id: `inner_${tag}`, type: 'mark', label: tag }], edges: [] }); +const SCREEN: Config = { fields: [{ name: 'tier', label: 'Tier', type: 'select', options: [{ label: 'Gold', value: 'gold' }] }] }; + +/** The census — the same rows `FlowSchema.parse` runs in `spec`. */ +const CENSUS: Array<{ type: string; config: Config; key: string }> = [ + { type: 'get_record', config: { objectName: 'account', outputVariable: 'rows' }, key: 'objectName' }, + { type: 'create_record', config: { objectName: 'account', fields: { name: 'Acme' } }, key: 'objectName' }, + { type: 'update_record', config: { objectName: 'account', filter: { id: '1' }, fields: { name: 'Acme' } }, key: 'objectName' }, + { type: 'delete_record', config: { objectName: 'account', filter: { id: '1' } }, key: 'objectName' }, + { type: 'notify', config: { recipients: ['u1'], title: 'Hi' }, key: 'recipients' }, + { type: 'http', config: { url: 'https://example.invalid/hook' }, key: 'url' }, + { type: 'screen', config: SCREEN, key: 'fields[0].name' }, + { type: 'screen', config: SCREEN, key: 'fields[0].options[0].value' }, + { type: 'screen', config: SCREEN, key: 'fields[0].options[0].label' }, + { type: 'script', config: { function: 'recalc_totals' }, key: 'function' }, + { type: 'subflow', config: { flowName: 'child_flow' }, key: 'flowName' }, + { type: 'map', config: { collection: [1], flowName: 'child_flow' }, key: 'collection' }, + { type: 'map', config: { collection: [1], flowName: 'child_flow' }, key: 'flowName' }, + { type: 'loop', config: { collection: [1], body: region('loop') }, key: 'collection' }, + { type: 'parallel', config: { branches: [region('p1'), region('p2')] }, key: 'branches' }, + { type: 'try_catch', config: { try: region('try'), catch: region('catch') }, key: 'try' }, +]; + +describe('registerFlow refuses a key the node\'s executor contract requires, left out (#20316)', () => { + it.each(CENSUS)('$type without `$key` does not register, and the refusal is the one judge\'s', ({ type, config, key }) => { + const engine = builtinEngine(); + const authored = without(config, key); + const refusal = refusalOf(engine, flowWith('census_probe', { id: 'n', type, label: 'N', config: authored })); + expect(refusal).toBeDefined(); + expect(refusal!.issues?.map((i) => [i.code, i.path, i.message])).toEqual([ + ['custom', ['nodes', 1, 'config', ...segmentsOf(key)], flowNodeConfigRefusals(type, authored)[0].message], + ]); + }); + + it.each(CENSUS)('$type with `$key` registers — the accept control', ({ type, config }) => { + const engine = builtinEngine(); + expect(refusalOf(engine, flowWith('census_probe', { id: 'n', type, label: 'N', config }))).toBeUndefined(); + }); + + it('CONTROL — a legacy flat-graph `loop` (no `body`, no `collection`) still registers and runs', async () => { + const engine = builtinEngine(); + expect(refusalOf(engine, flowWith('legacy_loop', { id: 'n', type: 'loop', label: 'N', config: {} }))).toBeUndefined(); + expect((await engine.execute('legacy_loop', { params: {} } as never)).success).toBe(true); + }); +}); + +/** + * What a refused shape DID at run time — the ground each refusal stands on. + * It can no longer register, so each run registers the whole config and + * deletes the key from the stored flow before executing, the way + * `decision-branch-expression-absent.test.ts` measures the absent predicate. + */ +describe('what the refused shapes did at run time (#20316)', () => { + async function runWithout(type: string, config: Config, key: string) { + const engine = builtinEngine(); + engine.registerFlow('child_flow', flowWith('child_flow') as never); + const parsed = engine.registerFlow('probe', flowWith('probe', { id: 'n', type, label: 'N', config }) as never); + const stored = parsed.nodes[1].config as Config; + const segments = segmentsOf(key); + let parent: any = stored; + for (const segment of segments.slice(0, -1)) parent = parent[segment]; + delete parent[segments[segments.length - 1]]; + return engine.execute('probe', { params: {} } as never); + } + + it.each([ + { type: 'loop', config: { collection: [1], body: region('loop') }, key: 'collection' }, + { type: 'map', config: { collection: [1], flowName: 'child_flow' }, key: 'collection' }, + { type: 'subflow', config: { flowName: 'child_flow' }, key: 'flowName' }, + { type: 'parallel', config: { branches: [region('p1'), region('p2')] }, key: 'branches' }, + ])('$type without `$key` failed the run at the node, as a contract refusal', async ({ type, config, key }) => { + const result = await runWithout(type, config, key); + expect(result.success).toBe(false); + expect(String(result.error)).toContain(`config does not satisfy the ${type} contract — config.${key}`); + }); + + async function routeWith(branch: Record) { + const visited: string[] = []; + const engine = builtinEngine(visited); + const parsed = engine.registerFlow('route', { + name: 'route', label: 'Route', type: 'autolaunched', + nodes: [ + { id: 'start', type: 'start', label: 'Start' }, + { id: 'd', type: 'decision', label: 'D', config: { conditions: [{ label: 'yes', expression: 'true' }] } }, + { id: 'y', type: 'mark', label: 'y' }, + { id: 'x', type: 'mark', label: 'x' }, + ], + edges: [ + { id: 'e0', source: 'start', target: 'd' }, + { id: 'e1', source: 'd', target: 'y', label: 'yes' }, + { id: 'e2', source: 'd', target: 'x', isDefault: true }, + ], + } as never); + (parsed.nodes[1].config as { conditions: unknown[] }).conditions[0] = branch; + const result = await engine.execute('route', { params: {} } as never); + return { success: result.success, visited: visited.sort() }; + } + + it('a matched branch with no `label` ran GREEN down EVERY out-edge — the labelled one and the default', async () => { + expect(await routeWith({ expression: 'true' })).toEqual({ success: true, visited: ['x', 'y'] }); + }); + + it('CONTROL — the same branch carrying its label takes only the out-edge it names', async () => { + expect(await routeWith({ label: 'yes', expression: 'true' })).toEqual({ success: true, visited: ['y'] }); + }); + + it('a branch that is a bare string failed the run at the node', async () => { + expect((await routeWith('true' as never)).success).toBe(false); + }); +}); + +describe('registerFlow refuses a decision branch list the executor cannot read (#20316)', () => { + const decision = (config: Config): Node => ({ id: 'd', type: 'decision', label: 'D', config }); + + it.each([ + ['a branch with no `label`', { conditions: [{ expression: 'true' }] }, ['conditions', 0, 'label']], + ['a branch with a blank `label`', { conditions: [{ label: ' ', expression: 'true' }] }, ['conditions', 0, 'label']], + ['a branch that is a string', { conditions: ['true'] }, ['conditions', 0]], + ['`conditions` that is an object', { conditions: {} }, ['conditions']], + ])('%s does not register', (_name, config, at) => { + const engine = builtinEngine(); + const refusal = refusalOf(engine, flowWith('decision_probe', decision(config))); + expect(refusal!.issues?.map((i) => [i.code, i.path, i.message])).toEqual([ + ['custom', ['nodes', 1, 'config', ...at], flowNodeConfigRefusals('decision', config)[0].message], + ]); + }); + + it('CONTROL — a labelled branch, and a decision with no branch list, register', () => { + expect(refusalOf(builtinEngine(), flowWith('decision_probe', decision({ conditions: [{ label: 'y', expression: 'true' }] })))).toBeUndefined(); + expect(refusalOf(builtinEngine(), flowWith('decision_probe', decision({})))).toBeUndefined(); + }); +}); diff --git a/packages/spec/src/automation/flow-node-config-required.test.ts b/packages/spec/src/automation/flow-node-config-required.test.ts new file mode 100644 index 00000000000..178759f21b4 --- /dev/null +++ b/packages/spec/src/automation/flow-node-config-required.test.ts @@ -0,0 +1,254 @@ +// Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license. + +/** + * #20316 — what a node's executor needs its `config` to carry is refused at + * `FlowSchema.parse`, the first of the three doors. + * + * A node's `config` is an open `z.record`, so two kinds of authored config + * passed `FlowSchema.parse`, `AutomationEngine.registerFlow` and + * `objectstack validate` and only met the run: + * + * - a key the node's EXECUTOR CONTRACT requires, left out — the executor's + * `parseNodeConfig` call refuses the node on every run that reaches it + * (measured for every contract-parsing builtin; the census is the table + * below, and `loop` with a `body` but no `collection` was the first one + * found); + * - a `decision` branch list the executor cannot read — a branch with no + * `label` never failed a run at all: the matched branch reported no label, + * and traversal took EVERY out-edge. A non-object branch, or a + * `conditions` that is not an array, failed the run at the node. + * + * Every refused row asserts the issue `code`, the `path` and the full message, + * read off the spec's own `flowNodeConfigRefusals` — never re-spelled — so the + * three doors are held to ONE judge. The other two doors run the same census + * in their own packages (`service-automation`'s + * `node-config-required-keys.test.ts`, `lint`'s `validate-expressions.test.ts`). + */ + +import { describe, expect, it } from 'vitest'; + +import { NotifyConfigSchema } from './io-node-config.zod'; +import { ScreenConfigSchema } from './builtin-node-config.zod'; +import { flowNodeConfigRefusals } from './flow-node-expression-paths'; +import { FlowSchema } from './flow.zod'; + +type Node = Record; +type Config = Record; + +/** start → → end, chained by unconditional edges. */ +function flowWith(...middle: Node[]) { + const nodes: Node[] = [{ id: 'start', type: 'start', label: 'Start' }, ...middle, { id: 'end', type: 'end', label: 'End' }]; + const edges = nodes.slice(1).map((n, i) => ({ id: `e${i}`, source: String(nodes[i].id), target: String(n.id) })); + return { name: 'config_probe', label: 'Config probe', type: 'autolaunched', nodes, edges }; +} + +const node = (type: string, config?: Config): Node => ({ + id: 'probe', type, label: 'Probe', ...(config === undefined ? {} : { config }), +}); + +/** A one-node region whose node id is unique to `tag`. */ +const region = (tag: string) => ({ nodes: [{ id: `inner_${tag}`, type: 'assignment', label: tag }], edges: [] }); + +function issuesOf(flow: unknown) { + const result = FlowSchema.safeParse(flow); + return result.success ? [] : result.error.issues; +} + +/** `fields[0].options[0].value` → `['fields', 0, 'options', 0, 'value']`. */ +function segmentsOf(path: string): (string | number)[] { + return path.split('.').flatMap((part) => { + const bracket = part.indexOf('['); + if (bracket === -1) return [part]; + return [part.slice(0, bracket), ...[...part.slice(bracket).matchAll(/\[(\d+)\]/g)].map((m) => Number(m[1]))]; + }); +} + +/** A deep copy of `config` without the key at `path`. */ +function without(config: Config, path: string): Config { + const copy = JSON.parse(JSON.stringify(config)) as Config; + const segments = segmentsOf(path); + let parent: any = copy; + for (const segment of segments.slice(0, -1)) parent = parent[segment]; + delete parent[segments[segments.length - 1]]; + return copy; +} + +const SCREEN: Config = { + fields: [{ name: 'tier', label: 'Tier', type: 'select', options: [{ label: 'Gold', value: 'gold' }] }], +}; + +/** + * The census: every key a contract-parsing builtin's executor contract + * requires. `config` is a whole, valid config — the accept control — and + * `key` the one left out. + */ +const CENSUS: Array<{ type: string; config: Config; key: string }> = [ + { type: 'get_record', config: { objectName: 'account', outputVariable: 'rows' }, key: 'objectName' }, + { type: 'create_record', config: { objectName: 'account', fields: { name: 'Acme' } }, key: 'objectName' }, + { type: 'update_record', config: { objectName: 'account', filter: { id: '{record.id}' }, fields: { name: 'Acme' } }, key: 'objectName' }, + { type: 'delete_record', config: { objectName: 'account', filter: { id: '{record.id}' } }, key: 'objectName' }, + { type: 'notify', config: { recipients: ['{record.owner}'], title: 'Hi' }, key: 'recipients' }, + { type: 'http', config: { url: 'https://example.com/hook' }, key: 'url' }, + { type: 'screen', config: SCREEN, key: 'fields[0].name' }, + { type: 'screen', config: SCREEN, key: 'fields[0].options[0].value' }, + { type: 'screen', config: SCREEN, key: 'fields[0].options[0].label' }, + { type: 'script', config: { function: 'recalc_totals' }, key: 'function' }, + { type: 'subflow', config: { flowName: 'child_flow' }, key: 'flowName' }, + { type: 'map', config: { collection: '{rows}', flowName: 'child_flow' }, key: 'collection' }, + { type: 'map', config: { collection: '{rows}', flowName: 'child_flow' }, key: 'flowName' }, + { type: 'loop', config: { collection: '{rows}', body: region('loop') }, key: 'collection' }, + { type: 'parallel', config: { branches: [region('p1'), region('p2')] }, key: 'branches' }, + { type: 'try_catch', config: { try: region('try'), catch: region('catch') }, key: 'try' }, +]; + +describe('FlowSchema.parse refuses a key the node\'s executor contract requires, left out (#20316)', () => { + it.each(CENSUS)('$type: the whole config parses — the accept control for `$key`', ({ type, config }) => { + expect(issuesOf(flowWith(node(type, config)))).toEqual([]); + }); + + it.each(CENSUS)('$type without `$key` is refused, anchored at the key', ({ type, config, key }) => { + const authored = without(config, key); + const issues = issuesOf(flowWith(node(type, authored))); + const judged = flowNodeConfigRefusals(type, authored); + expect(judged.map((r) => [r.code, r.path, r.params])).toEqual([['node-config-key-missing', key, { nodeType: type, key }]]); + expect(issues.map((i) => [i.code, i.path, i.message])).toEqual([ + ['custom', ['nodes', 1, 'config', ...segmentsOf(key)], judged[0].message], + ]); + expect(issues[0].message).toContain(`This \`${type}\` node's config leaves out \`${key}\``); + }); + + it('a node with no `config` at all is judged as the executor reads it — `{}`', () => { + const issues = issuesOf(flowWith(node('http'))); + expect(issues.map((i) => [i.code, i.path])).toEqual([['custom', ['nodes', 1, 'config', 'url']]]); + }); + + describe('a key a RULE of the contract requires here — the contract\'s own message', () => { + it('a `notify` with neither `title` nor `template` is refused at `title`, in the notify contract\'s words', () => { + const authored = { recipients: ['{record.owner}'] }; + const own = NotifyConfigSchema.safeParse(authored); + expect(own.success).toBe(false); + const issues = issuesOf(flowWith(node('notify', authored))); + expect(issues.map((i) => [i.code, i.path, i.message])).toEqual([ + ['custom', ['nodes', 1, 'config', 'title'], own.error!.issues.find((i) => i.path.join('.') === 'title')!.message], + ]); + expect(flowNodeConfigRefusals('notify', authored).map((r) => r.code)).toEqual(['node-config-key-required-by-rule']); + }); + + it('a `lookup` screen field with no `reference` is refused at the field\'s `reference`, in the screen contract\'s words', () => { + const authored = { fields: [{ name: 'account', label: 'Account', type: 'lookup' }] }; + const own = ScreenConfigSchema.safeParse(authored); + expect(own.success).toBe(false); + const issues = issuesOf(flowWith(node('screen', authored))); + expect(issues.map((i) => [i.code, i.path, i.message])).toEqual([ + ['custom', ['nodes', 1, 'config', 'fields', 0, 'reference'], own.error!.issues[0].message], + ]); + expect(flowNodeConfigRefusals('screen', authored).map((r) => r.code)).toEqual(['node-config-key-required-by-rule']); + }); + }); + + it('reaches a node inside an ADR-0031 region body, anchored at the inner node — never re-reported against the container', () => { + const inner = { id: 'fetch', type: 'get_record', label: 'Fetch', config: { outputVariable: 'rows' } }; + const issues = issuesOf(flowWith(node('loop', { collection: '{rows}', body: { nodes: [inner], edges: [] } }))); + expect(issues.map((i) => [i.code, i.path])).toEqual([ + ['custom', ['nodes', 1, 'config', 'body', 'nodes', 0, 'config', 'objectName']], + ]); + }); + + describe('CONTROLS — what this rule must NOT reach', () => { + it('a legacy flat-graph `loop` (no `body`) needs no `collection` — its executor does not parse it', () => { + expect(issuesOf(flowWith(node('loop', {})))).toEqual([]); + expect(issuesOf(flowWith(node('loop')))).toEqual([]); + }); + + it('a PRESENT value of the wrong type is not this rule\'s finding — absence only', () => { + expect(issuesOf(flowWith(node('get_record', { objectName: 42 })))).toEqual([]); + expect(issuesOf(flowWith(node('script', { function: '' })))).toEqual([]); + }); + + it('an undeclared key is not this rule\'s finding — no key-set closure', () => { + expect(issuesOf(flowWith(node('http', { url: 'https://example.com', zzz_undeclared: 1 })))).toEqual([]); + }); + + it('a node type with no executor contract is untouched — `assignment`, a plugin type', () => { + expect(issuesOf(flowWith(node('assignment', {})))).toEqual([]); + expect(issuesOf(flowWith(node('acme_custom_step', {})))).toEqual([]); + }); + }); +}); + +/** A decision whose `config` is written exactly as given. */ +const decision = (config: Config): Node => ({ id: 'check', type: 'decision', label: 'Check', config }); + +describe('FlowSchema.parse refuses a decision branch list the executor cannot read (#20316)', () => { + const AT = (...rest: (string | number)[]) => ['nodes', 1, 'config', ...rest]; + + it.each([ + ['nothing — the key is absent', { expression: 'true' }, 'absent'], + ['`null`', { label: null, expression: 'true' }, 'null'], + ['an empty string', { label: '', expression: 'true' }, 'blank'], + ['a string blank after trimming', { label: ' ', expression: 'true' }, 'blank'], + ['a number', { label: 42, expression: 'true' }, 'number'], + ['a boolean', { label: false, expression: 'true' }, 'boolean'], + ])('a branch whose `label` holds %s is refused at the label', (_name, branch, found) => { + const config = { conditions: [{ label: 'first', expression: 'false' }, branch] }; + const issues = issuesOf(flowWith(decision(config))); + const judged = flowNodeConfigRefusals('decision', config); + expect(judged.map((r) => [r.code, r.params])).toEqual([['decision-branch-label-missing', { index: 1, found }]]); + expect(issues.map((i) => [i.code, i.path, i.message])).toEqual([['custom', AT('conditions', 1, 'label'), judged[0].message]]); + }); + + it.each([ + ['a string — the bare predicate', 'true', 'string'], + ['`null`', null, 'null'], + ['a number', 42, 'number'], + ['an array', ['true'], 'array'], + ])('a branch that is %s is refused as a branch, once', (_name, branch, found) => { + const config = { conditions: [branch] }; + const issues = issuesOf(flowWith(decision(config))); + const judged = flowNodeConfigRefusals('decision', config); + expect(judged.map((r) => [r.code, r.params])).toEqual([['decision-branch-not-object', { index: 0, found }]]); + // ONE issue: an array element is not read as an object missing its + // `expression` too — the expression walk does not reach it. + expect(issues.map((i) => [i.code, i.path, i.message])).toEqual([['custom', AT('conditions', 0), judged[0].message]]); + }); + + it.each([ + ['an object', {}, 'object'], + ['a string', 'true', 'string'], + ['a number', 5, 'number'], + ])('`conditions` holding %s is refused at `conditions`', (_name, conditions, found) => { + const config = { conditions }; + const issues = issuesOf(flowWith(decision(config))); + const judged = flowNodeConfigRefusals('decision', config); + expect(judged.map((r) => [r.code, r.params])).toEqual([['decision-conditions-not-array', { found }]]); + expect(issues.map((i) => [i.code, i.path, i.message])).toEqual([['custom', AT('conditions'), judged[0].message]]); + }); + + it('an empty branch is refused twice, once per key — the label here, the expression by its own slot rule', () => { + const issues = issuesOf(flowWith(decision({ conditions: [{}] }))); + expect(issues.map((i) => i.path)).toEqual([AT('conditions', 0, 'expression'), AT('conditions', 0, 'label')]); + }); + + it('reaches a decision inside a region body, anchored where the author wrote it', () => { + const issues = issuesOf(flowWith(node('loop', { + collection: '{rows}', + body: { nodes: [{ id: 'inner', type: 'decision', label: 'Inner', config: { conditions: [{ expression: 'true' }] } }], edges: [] }, + }))); + expect(issues.map((i) => [i.code, i.path])).toEqual([ + ['custom', ['nodes', 1, 'config', 'body', 'nodes', 0, 'config', 'conditions', 0, 'label']], + ]); + }); + + describe('CONTROLS', () => { + it('a labelled branch parses', () => { + expect(issuesOf(flowWith(decision({ conditions: [{ label: 'yes', expression: 'true' }] })))).toEqual([]); + }); + + it('a decision that declares no branch still parses — absent, `null`, or empty — it routes by its out-edges', () => { + expect(issuesOf(flowWith(decision({})))).toEqual([]); + expect(issuesOf(flowWith(decision({ conditions: null })))).toEqual([]); + expect(issuesOf(flowWith(decision({ conditions: [] })))).toEqual([]); + expect(issuesOf(flowWith({ id: 'check', type: 'decision', label: 'Check' }))).toEqual([]); + }); + }); +}); diff --git a/packages/spec/src/automation/flow-node-expression-paths.ts b/packages/spec/src/automation/flow-node-expression-paths.ts index 28a395eae5f..b3bdef23a2f 100644 --- a/packages/spec/src/automation/flow-node-expression-paths.ts +++ b/packages/spec/src/automation/flow-node-expression-paths.ts @@ -1224,7 +1224,7 @@ function decisionShapeRefusals(config: unknown, out: FlowNodeConfigRefusal[]): v params: { index, found }, message: 'A decision branch routes by its `label`: the first branch whose `expression` holds is taken, and the run ' - + `continues down the out-edge carrying that label. \`${path}.label\` holds ${phrase}, which names no ` + + `continues down the out-edge carrying that label. \`${path}.label\` holds ${phrase}, and that names no ` + 'out-edge — so when this branch matches, the node reports no branch it can route, and traversal ' + 'considers EVERY out-edge instead, as if the decision declared no branches: an unconditional labelled ' + 'out-edge and the default out-edge both run. Write the label of the out-edge this branch should take ' diff --git a/packages/spec/src/automation/flow-slot-refusal-codes.test.ts b/packages/spec/src/automation/flow-slot-refusal-codes.test.ts index 84a1e359527..90e07f21cc1 100644 --- a/packages/spec/src/automation/flow-slot-refusal-codes.test.ts +++ b/packages/spec/src/automation/flow-slot-refusal-codes.test.ts @@ -1,8 +1,9 @@ // Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license. /** - * The refusal codes this file's two refusal producers — `predicateSlotRefusal` - * and `structuralConditionRefusal` — carry beside their English message. + * The refusal codes this file's three refusal producers — `predicateSlotRefusal`, + * `structuralConditionRefusal` and (#20316) `flowNodeConfigRefusals` — carry + * beside their English message. * * A localized designer keys its own catalogue row to the `code` and fills it * from the `params`; API callers, `registerFlow` and `objectstack validate` @@ -24,8 +25,11 @@ import { FLOW_SLOT_REFUSAL_CODES, PREDICATE_SLOT_STRING_REFUSAL, STRUCTURAL_CONDITION_SHAPE_REFUSAL, + flowNodeConfigRefusals, predicateSlotRefusal, structuralConditionRefusal, + type FlowNodeConfigRefusal, + type FlowNodeConfigRefusalCode, type FlowSlotRefusalCode, type FlowSlotRefusalParams, type PredicateSlotRefusal, @@ -34,6 +38,7 @@ import { type StructuralConditionRefusalCode, } from './flow-node-expression-paths.js'; import * as automation from './index.js'; +import { NotifyConfigSchema } from './io-node-config.zod.js'; /** What one refusal says, whichever producer said it. */ interface Said { @@ -52,6 +57,45 @@ interface Pin { const slot = (value: unknown) => (): Said | undefined => predicateSlotRefusal(value); const structural = (value: unknown) => (): Said | undefined => structuralConditionRefusal(value); +/** The ONE refusal a node config provokes — two would make the pin ambiguous. */ +const nodeConfig = (nodeType: string, config: unknown) => (): Said | undefined => { + const refusals = flowNodeConfigRefusals(nodeType, config); + expect(refusals).toHaveLength(1); + return refusals[0]; +}; + +const CONDITIONS_NOT_ARRAY = (found: string): string => + `A decision's \`conditions\` is its ordered branch list — an array of \`{ label, expression }\` — and this one is ${found}. ` + + 'The decision executor iterates it, so a run that reaches the node fails there (a string is iterated character ' + + 'by character, each character a branch with no `expression`). Write the branches as an array, or delete ' + + '`conditions` and route by the out-edges\' own `condition`s.'; + +const BRANCH_NOT_OBJECT = (path: string, found: string): string => + `A decision branch is an object — \`{ label, expression }\` — and \`${path}\` is ${found}. The decision executor ` + + 'reads `label` and `expression` off every branch it reaches, so this one has neither and a run that reaches it ' + + 'fails at the node. Write it as `{ label: \'approved\', expression: \'record.amount > 1000\' }` — the label of ' + + 'the out-edge it routes to, and a bare CEL predicate; a predicate written as a bare string belongs under ' + + '`expression`.'; + +const LABEL_MISSING = (path: string, found: string): string => + 'A decision branch routes by its `label`: the first branch whose `expression` holds is taken, and the run continues ' + + `down the out-edge carrying that label. \`${path}\` holds ${found}, and that names no out-edge — so when this ` + + 'branch matches, the node reports no branch it can route, and traversal considers EVERY out-edge instead, as if ' + + 'the decision declared no branches: an unconditional labelled out-edge and the default out-edge both run. Write ' + + 'the label of the out-edge this branch should take (`label: \'approved\'` for the out-edge labelled `approved`). ' + + 'To branch on the out-edges instead, delete `conditions` and put each predicate on its edge\'s `condition`.'; + +const KEY_MISSING = (nodeType: string, key: string): string => + `This \`${nodeType}\` node's config leaves out \`${key}\`, which the ${nodeType} contract requires. Its executor ` + + 'parses the config against that contract before it does anything else and refuses the node without it — so the ' + + 'flow registers, and then every run that reaches this node fails there; the config is metadata, and re-running ' + + `changes nothing. Write \`${key}\` on the node's \`config\`.`; + +/** The notify contract's own words for a node with no content source — read, never re-spelled. */ +const NOTIFY_TITLE_RULE = (() => { + const own = NotifyConfigSchema.safeParse({ recipients: ['u1'] }); + return own.success ? '' : own.error.issues.find((i) => i.path.join('.') === 'title')?.message ?? ''; +})(); const MISSING_TAIL = ' where the slot is required: a decision branch is `{ label, expression }` and its `expression` is not optional, ' @@ -158,6 +202,97 @@ const PINS: { readonly [C in FlowSlotRefusalCode]: readonly [Pin, ...Pin[] { produce: structural(Symbol('s')), params: { found: 'symbol' }, message: shape('a symbol'), source: '' }, { produce: structural(() => 1), params: { found: 'function' }, message: shape('a function'), source: '' }, ], + 'decision-conditions-not-array': [ + { produce: nodeConfig('decision', { conditions: {} }), params: { found: 'object' }, message: CONDITIONS_NOT_ARRAY('an object'), source: '' }, + { produce: nodeConfig('decision', { conditions: 'true' }), params: { found: 'string' }, message: CONDITIONS_NOT_ARRAY('a string'), source: '' }, + { produce: nodeConfig('decision', { conditions: 5 }), params: { found: 'number' }, message: CONDITIONS_NOT_ARRAY('a number'), source: '' }, + ], + 'decision-branch-not-object': [ + { + produce: nodeConfig('decision', { conditions: ['true'] }), + params: { index: 0, found: 'string' }, + message: BRANCH_NOT_OBJECT('conditions[0]', 'a string'), + source: '', + }, + { + produce: nodeConfig('decision', { conditions: [{ label: 'a', expression: 'x' }, null] }), + params: { index: 1, found: 'null' }, + message: BRANCH_NOT_OBJECT('conditions[1]', '`null`'), + source: '', + }, + { + produce: nodeConfig('decision', { conditions: [['true']] }), + params: { index: 0, found: 'array' }, + message: BRANCH_NOT_OBJECT('conditions[0]', 'an array'), + source: '', + }, + { + produce: nodeConfig('decision', { conditions: [42] }), + params: { index: 0, found: 'number' }, + message: BRANCH_NOT_OBJECT('conditions[0]', 'a number'), + source: '', + }, + ], + 'decision-branch-label-missing': [ + { + produce: nodeConfig('decision', { conditions: [{ expression: 'true' }] }), + params: { index: 0, found: 'absent' }, + message: LABEL_MISSING('conditions[0].label', 'nothing — the key is absent'), + source: '', + }, + { + produce: nodeConfig('decision', { conditions: [{ label: null, expression: 'true' }] }), + params: { index: 0, found: 'null' }, + message: LABEL_MISSING('conditions[0].label', '`null`'), + source: '', + }, + { + produce: nodeConfig('decision', { conditions: [{ label: 'a', expression: 'x' }, { label: ' \t', expression: 'true' }] }), + params: { index: 1, found: 'blank' }, + message: LABEL_MISSING('conditions[1].label', 'a string that is blank after trimming'), + source: '', + }, + { + produce: nodeConfig('decision', { conditions: [{ label: 42, expression: 'true' }] }), + params: { index: 0, found: 'number' }, + message: LABEL_MISSING('conditions[0].label', 'a number'), + source: '', + }, + { + produce: nodeConfig('decision', { conditions: [{ label: { text: 'yes' }, expression: 'true' }] }), + params: { index: 0, found: 'object' }, + message: LABEL_MISSING('conditions[0].label', 'an object'), + source: '', + }, + ], + 'node-config-key-missing': [ + { + produce: nodeConfig('loop', { iteratorVariable: 'row', body: { nodes: [{ id: 'b', type: 'assignment', label: 'B' }], edges: [] } }), + params: { nodeType: 'loop', key: 'collection' }, + message: KEY_MISSING('loop', 'collection'), + source: '', + }, + { + produce: nodeConfig('map', { flowName: 'child' }), + params: { nodeType: 'map', key: 'collection' }, + message: KEY_MISSING('map', 'collection'), + source: '', + }, + { + produce: nodeConfig('screen', { fields: [{ label: 'Tier' }] }), + params: { nodeType: 'screen', key: 'fields[0].name' }, + message: KEY_MISSING('screen', 'fields[0].name'), + source: '', + }, + ], + 'node-config-key-required-by-rule': [ + { + produce: nodeConfig('notify', { recipients: ['u1'] }), + params: { nodeType: 'notify', key: 'title' }, + message: NOTIFY_TITLE_RULE, + source: '', + }, + ], }; describe('flow slot refusal codes — one pin per code (code, params, unchanged message)', () => { @@ -175,6 +310,10 @@ describe('flow slot refusal codes — one pin per code (code, params, unchanged }); } + it('the rule-required pin reads a real sentence off the notify contract, not an empty one', () => { + expect(NOTIFY_TITLE_RULE.length).toBeGreaterThan(40); + }); + it('the structural lead sentence is the published constant, byte for byte', () => { expect(STRUCTURAL_CONDITION_SHAPE_REFUSAL).toBe(STRUCTURAL_LEAD); }); @@ -217,6 +356,31 @@ const PREDICATE_SLOT_CODES: ReadonlySet = new Set = new Set([ 'structural-condition-shape', ]); +const NODE_CONFIG_CODES: ReadonlySet = new Set([ + 'decision-conditions-not-array', + 'decision-branch-not-object', + 'decision-branch-label-missing', + 'node-config-key-missing', + 'node-config-key-required-by-rule', +]); + +/** Node configs of every shape, per node type — the sweep judges whatever each one provokes. */ +const CONFIG_SWEEP: ReadonlyArray = [ + ...SWEEP.map((value) => ['decision', { conditions: value }] as const), + ...SWEEP.map((value) => ['decision', { conditions: [value] }] as const), + ...SWEEP.map((value) => ['decision', { conditions: [{ label: value, expression: 'true' }] }] as const), + ['decision', undefined], + ['decision', {}], + ['get_record', {}], + ['get_record', undefined], + ['get_record', { objectName: 'account' }], + ['notify', { recipients: ['u1'] }], + ['notify', {}], + ['loop', {}], + ['loop', { body: { nodes: [{ id: 'b', type: 'assignment', label: 'B' }], edges: [] } }], + ['screen', { fields: [{ label: 'x', options: [{}] }] }], + ['assignment', {}], +]; describe('flow slot refusal codes — the closed set', () => { it('has a pin for every code, and no pin for a code outside the set', () => { @@ -225,8 +389,25 @@ describe('flow slot refusal codes — the closed set', () => { for (const code of FLOW_SLOT_REFUSAL_CODES) expect(code).toMatch(/^[a-z]+(?:-[a-z]+)+$/); }); - it('splits between the two producers with nothing left over', () => { - expect([...PREDICATE_SLOT_CODES, ...STRUCTURAL_CODES].sort()).toEqual([...FLOW_SLOT_REFUSAL_CODES].sort()); + it('splits between the three producers with nothing left over', () => { + expect([...PREDICATE_SLOT_CODES, ...STRUCTURAL_CODES, ...NODE_CONFIG_CODES].sort()).toEqual([...FLOW_SLOT_REFUSAL_CODES].sort()); + }); + + it('every flowNodeConfigRefusals on the sweep carries a node-config code and a path, and every code is reached', () => { + const provoked = new Set(); + let refused = 0; + for (const [nodeType, config] of CONFIG_SWEEP) { + for (const refusal of flowNodeConfigRefusals(nodeType, config)) { + refused++; + expect(NODE_CONFIG_CODES.has(refusal.code)).toBe(true); + expect(typeof refusal.params).toBe('object'); + expect(refusal.path.length).toBeGreaterThan(0); + expect(refusal.source).toBe(''); + provoked.add(refusal.code); + } + } + expect(refused).toBeGreaterThan(40); + expect([...provoked].sort()).toEqual([...NODE_CONFIG_CODES].sort()); }); it('is published from the automation entry, frozen', () => { @@ -284,6 +465,10 @@ describe('flow slot refusal codes — the closed set', () => { const noStructuralCode: StructuralConditionRefusal = { message: 'm', source: '' }; // @ts-expect-error — each producer emits only its own codes: a structural refusal is never a predicate-slot code. const foreignCode: StructuralConditionRefusal = { message: 'm', source: '', code: 'predicate-slot-blank', params: {} }; - expect([noCode, wrongParams, noStructuralCode, foreignCode]).toHaveLength(4); + // @ts-expect-error — a node config refusal carries its `path` inside the config. + const noPath: FlowNodeConfigRefusal = { message: 'm', source: '', code: 'node-config-key-missing', params: { nodeType: 'loop', key: 'collection' } }; + // @ts-expect-error — `node-config-key-missing` names the node type and the key, never a `found`. + const wrongNodeParams: FlowNodeConfigRefusal = { message: 'm', source: '', path: 'x', code: 'node-config-key-missing', params: { found: 'absent' } }; + expect([noCode, wrongParams, noStructuralCode, foreignCode, noPath, wrongNodeParams]).toHaveLength(6); }); }); diff --git a/packages/spec/src/migrations/entries/semantic/18.flow-node-config-required-keys-refused.ts b/packages/spec/src/migrations/entries/semantic/18.flow-node-config-required-keys-refused.ts new file mode 100644 index 00000000000..4dbf12ea432 --- /dev/null +++ b/packages/spec/src/migrations/entries/semantic/18.flow-node-config-required-keys-refused.ts @@ -0,0 +1,74 @@ +// Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license. + +import type { SemanticMigration } from '../../types.js'; + +// ONE entry for the family, not one per node type: every member is the same +// decision — a node config its executor cannot run is refused where the flow +// is built, by one judge (`flowNodeConfigRefusals`), instead of registering +// and failing (or, for a branch with no label, misrouting) at run time. +// +// Form D: no tracker number anywhere in the author-shown text; the decision is +// stated in words. +// +// No backticks in `surface` — build-upgrade-guide.ts renders it inside a code +// span already, and a nested backtick would close it. +export const entry: SemanticMigration = { + id: 'flow-node-config-required-keys-refused', + surface: + 'a flow node whose config leaves out a key its executor contract requires — objectName on ' + + 'get_record / create_record / update_record / delete_record, recipients on notify (and title ' + + 'when there is no template), url on http, function on script, flowName on subflow, collection ' + + 'and flowName on map, collection on a loop that has a body, branches on parallel, try on ' + + 'try_catch, and on screen each field name, each option value and label, and a lookup field ' + + 'reference — and a decision node whose conditions is not an array, holds a branch that is not ' + + 'an object, or holds a branch whose label is absent, null, blank or not a string; at any depth ' + + 'including an ADR-0031 region body. Reachable wherever a flow is authored or stored: ' + + 'defineStack({ flows }) sources, defineFlow(), an exported stack passed to objectstack validate, ' + + 'a flow saved from the Studio flow designer (a node added and saved before it is configured; a ' + + 'decision branch row whose label cell is empty; a screen field row whose name cell is empty), ' + + 'and a flow row already sitting in sys_metadata', + replacement: + 'the missing key, written on the node\'s `config` — the value the node was meant to act on ' + + '(`objectName: \'account\'`, `url: \'https://…\'`, `collection: \'{rows}\'`, …). For a decision ' + + 'branch, the label of the out-edge the branch should take (`{ label: \'approved\', expression: ' + + '\'record.amount > 1000\' }`, beside an out-edge labelled `approved`), `conditions` written as an ' + + 'array of such objects, and a bare predicate string moved under `expression`. To branch on the ' + + 'out-edges instead, delete `conditions` and put each predicate on its edge\'s `condition`. A ' + + 'legacy flat-graph `loop` (no `body`) needs no `collection` and is untouched', + reason: + 'A flow node\'s `config` is an open record, so what its executor requires was checked by no build ' + + 'door: `FlowSchema.parse`, `AutomationEngine.registerFlow` and `objectstack validate` all admitted ' + + 'a node missing a key its executor contract requires, and the executor\'s own contract parse then ' + + 'refused the node on every run that reached it — the config is metadata, so no rerun could ' + + 'succeed. A decision branch with no label was worse: it never failed, the matched branch reported ' + + 'no label and traversal took EVERY out-edge, so the flow ran green down the wrong paths. All ' + + 'three doors now refuse these shapes through one judge, `flowNodeConfigRefusals`, which parses ' + + 'each builtin node\'s config against the very contract its executor parses against ' + + '(`getBuiltinNodeConfigContracts()`, reconciled against the executors\' own parse calls) and keeps ' + + 'only the keys left out — a present value of the wrong type and an undeclared key are judged where ' + + 'they were before — plus the decision branch shape its executor reads raw. A key a rule of the ' + + 'contract requires (a notify with no template needs a title; a lookup screen field needs its ' + + 'reference) is refused in the contract\'s own words. ' + + '⚠️ No D2 conversion: the platform cannot know the object, URL, collection, function or ' + + 'out-edge label the author left out, and no value it could write would keep what the flow did. ' + + '⚠️ Where such a node already sits the whole flow is refused: registered from the metadata ' + + 'registry or `sys_metadata` at boot it is skipped with a `warn` naming it, its trigger not armed, ' + + 'while the flows beside it register; a `defineStack({ flows })` source throws ' + + '`StackSchemaInvalidError` for the whole stack; an artifact file is refused whole at load. ' + + 'ADR-0087, ADR-0031.', + acceptanceCriteria: + 'Run `objectstack validate` over every stack authored in config files, and boot every deployed ' + + 'stack. Each refusal names the node and the key: `FlowSchema.parse` anchors a `custom` issue at ' + + '`nodes.N.config.` (`nodes.N.config.fields.0.name`, `nodes.N.config.conditions.0.label`, or ' + + 'the region path `nodes.N.config.body.nodes.M.config…`), `objectstack validate` prints the same ' + + 'path, and `validateStackExpressions` phrases it as `node \'fetch\' (get_record) config.objectName`. ' + + 'For each hit write the key the node was meant to carry, per the replacement. Two proofs. (1) For ' + + 'a stack authored in config files, `objectstack validate` is clean. (2) Boot the stack and confirm ' + + 'each flow REGISTERS: no `failed to register flow` warn for it (the three boot paths spell it ' + + '`[Automation] failed to register flow`, `[Automation] flow re-sync: failed to register flow` and ' + + '`[Automation] cold-boot flow bind: failed to register flow`) — that warn line is the locator for a ' + + 'row that exists only in `sys_metadata`. A node carrying every key its contract requires parses ' + + 'and registers byte-identically to before, a decision with no `conditions` (or `conditions: null`, ' + + 'or an empty list) still routes by its out-edges, and a legacy `loop` with no `body` still needs ' + + 'no `collection`.', +}; diff --git a/packages/spec/src/migrations/registry.ts b/packages/spec/src/migrations/registry.ts index e7ff7c80d24..a90cc66e887 100644 --- a/packages/spec/src/migrations/registry.ts +++ b/packages/spec/src/migrations/registry.ts @@ -10435,6 +10435,76 @@ const step18: MigrationStep = { + "slot phrase. A flow that boots without that warn is unaffected; every structural " + 'condition carrying a non-blank `source` parses byte-identically to before.', }, + // ONE entry for the family, not one per node type: every member is the same + // decision — a node config its executor cannot run is refused where the flow + // is built, by one judge (`flowNodeConfigRefusals`), instead of registering + // and failing (or, for a branch with no label, misrouting) at run time. + // + // Form D: no tracker number anywhere in the author-shown text; the decision is + // stated in words. + // + // No backticks in `surface` — build-upgrade-guide.ts renders it inside a code + // span already, and a nested backtick would close it. + { + id: 'flow-node-config-required-keys-refused', + surface: + 'a flow node whose config leaves out a key its executor contract requires — objectName on ' + + 'get_record / create_record / update_record / delete_record, recipients on notify (and title ' + + 'when there is no template), url on http, function on script, flowName on subflow, collection ' + + 'and flowName on map, collection on a loop that has a body, branches on parallel, try on ' + + 'try_catch, and on screen each field name, each option value and label, and a lookup field ' + + 'reference — and a decision node whose conditions is not an array, holds a branch that is not ' + + 'an object, or holds a branch whose label is absent, null, blank or not a string; at any depth ' + + 'including an ADR-0031 region body. Reachable wherever a flow is authored or stored: ' + + 'defineStack({ flows }) sources, defineFlow(), an exported stack passed to objectstack validate, ' + + 'a flow saved from the Studio flow designer (a node added and saved before it is configured; a ' + + 'decision branch row whose label cell is empty; a screen field row whose name cell is empty), ' + + 'and a flow row already sitting in sys_metadata', + replacement: + 'the missing key, written on the node\'s `config` — the value the node was meant to act on ' + + '(`objectName: \'account\'`, `url: \'https://…\'`, `collection: \'{rows}\'`, …). For a decision ' + + 'branch, the label of the out-edge the branch should take (`{ label: \'approved\', expression: ' + + '\'record.amount > 1000\' }`, beside an out-edge labelled `approved`), `conditions` written as an ' + + 'array of such objects, and a bare predicate string moved under `expression`. To branch on the ' + + 'out-edges instead, delete `conditions` and put each predicate on its edge\'s `condition`. A ' + + 'legacy flat-graph `loop` (no `body`) needs no `collection` and is untouched', + reason: + 'A flow node\'s `config` is an open record, so what its executor requires was checked by no build ' + + 'door: `FlowSchema.parse`, `AutomationEngine.registerFlow` and `objectstack validate` all admitted ' + + 'a node missing a key its executor contract requires, and the executor\'s own contract parse then ' + + 'refused the node on every run that reached it — the config is metadata, so no rerun could ' + + 'succeed. A decision branch with no label was worse: it never failed, the matched branch reported ' + + 'no label and traversal took EVERY out-edge, so the flow ran green down the wrong paths. All ' + + 'three doors now refuse these shapes through one judge, `flowNodeConfigRefusals`, which parses ' + + 'each builtin node\'s config against the very contract its executor parses against ' + + '(`getBuiltinNodeConfigContracts()`, reconciled against the executors\' own parse calls) and keeps ' + + 'only the keys left out — a present value of the wrong type and an undeclared key are judged where ' + + 'they were before — plus the decision branch shape its executor reads raw. A key a rule of the ' + + 'contract requires (a notify with no template needs a title; a lookup screen field needs its ' + + 'reference) is refused in the contract\'s own words. ' + + '⚠️ No D2 conversion: the platform cannot know the object, URL, collection, function or ' + + 'out-edge label the author left out, and no value it could write would keep what the flow did. ' + + '⚠️ Where such a node already sits the whole flow is refused: registered from the metadata ' + + 'registry or `sys_metadata` at boot it is skipped with a `warn` naming it, its trigger not armed, ' + + 'while the flows beside it register; a `defineStack({ flows })` source throws ' + + '`StackSchemaInvalidError` for the whole stack; an artifact file is refused whole at load. ' + + 'ADR-0087, ADR-0031.', + acceptanceCriteria: + 'Run `objectstack validate` over every stack authored in config files, and boot every deployed ' + + 'stack. Each refusal names the node and the key: `FlowSchema.parse` anchors a `custom` issue at ' + + '`nodes.N.config.` (`nodes.N.config.fields.0.name`, `nodes.N.config.conditions.0.label`, or ' + + 'the region path `nodes.N.config.body.nodes.M.config…`), `objectstack validate` prints the same ' + + 'path, and `validateStackExpressions` phrases it as `node \'fetch\' (get_record) config.objectName`. ' + + 'For each hit write the key the node was meant to carry, per the replacement. Two proofs. (1) For ' + + 'a stack authored in config files, `objectstack validate` is clean. (2) Boot the stack and confirm ' + + 'each flow REGISTERS: no `failed to register flow` warn for it (the three boot paths spell it ' + + '`[Automation] failed to register flow`, `[Automation] flow re-sync: failed to register flow` and ' + + '`[Automation] cold-boot flow bind: failed to register flow`) — that warn line is the locator for a ' + + 'row that exists only in `sys_metadata`. A node carrying every key its contract requires parses ' + + 'and registers byte-identically to before, a decision with no `conditions` (or `conditions: null`, ' + + 'or an empty list) still routes by its out-edges, and a legacy `loop` with no `body` still needs ' + + 'no `collection`.', + }, // The ledger `predicate` slots' half of the blank-predicate rule. A SEPARATE // entry from `flow-edge-condition-evaluated-slot-source-required` on purpose: // that one carries the structural slots (`edges[].condition`, From 5bf47a9ef6438dbf67ec4719e9c2abe38c3b4c78 Mon Sep 17 00:00:00 2001 From: Claude Date: Mon, 28 Sep 2026 05:12:13 +0000 Subject: [PATCH 3/8] wip: move the judge beside the walk, complete the fixtures the census refuses Claude-Session: https://claude.ai/code/session_01QcAS3qiYYZNezaxZxaUdMV Co-Authored-By: Claude --- .../contained-failure-visibility.test.ts | 6 +- .../service-automation/src/engine.test.ts | 42 +-- .../src/flow-terminal-messages.test.ts | 2 +- .../spec/src/api/zod-issues-to-fields.test.ts | 4 +- .../automation/flow-node-config-refusals.ts | 325 +++++++++++++++++ .../flow-node-config-required.test.ts | 2 +- .../automation/flow-node-expression-paths.ts | 333 +----------------- .../flow-region-pause-and-end.test.ts | 11 +- .../flow-slot-refusal-codes.test.ts | 2 +- packages/spec/src/automation/flow.test.ts | 17 +- packages/spec/src/automation/flow.zod.ts | 3 +- packages/spec/src/automation/index.ts | 1 + .../automation/region-normalization.test.ts | 21 +- .../spec/src/conversions/conversions.test.ts | 14 +- 14 files changed, 419 insertions(+), 364 deletions(-) create mode 100644 packages/spec/src/automation/flow-node-config-refusals.ts diff --git a/packages/services/service-automation/src/builtin/contained-failure-visibility.test.ts b/packages/services/service-automation/src/builtin/contained-failure-visibility.test.ts index d1bd556973a..b34227cd216 100644 --- a/packages/services/service-automation/src/builtin/contained-failure-visibility.test.ts +++ b/packages/services/service-automation/src/builtin/contained-failure-visibility.test.ts @@ -144,7 +144,7 @@ describe('#14456 — a contained per-iteration failure is visible, attributed an try: { nodes: [ { id: 'flag', type: 'flag', label: 'Flag' }, - { id: 'notify', type: 'notify', label: 'Notify' }, + { id: 'notify', type: 'notify', label: 'Notify', config: { title: 'Notice', recipients: ['user_1'] } }, ], edges: [{ id: 't1', source: 'flag', target: 'notify' }], }, @@ -256,7 +256,7 @@ describe('#14456 — a contained per-iteration failure is visible, attributed an { id: 'guard', type: 'try_catch', label: 'Guarded', config: { - try: { nodes: [{ id: 'notify', type: 'notify', label: 'Notify' }], edges: [] }, + try: { nodes: [{ id: 'notify', type: 'notify', label: 'Notify', config: { title: 'Notice', recipients: ['user_1'] } }], edges: [] }, catch: { nodes: [{ id: 'seen', type: 'capture', label: 'Seen' }], edges: [] }, }, }, @@ -287,7 +287,7 @@ describe('#14456 — a contained per-iteration failure is visible, attributed an { id: 'guard', type: 'try_catch', label: 'Guarded', config: { - try: { nodes: [{ id: 'notify', type: 'notify', label: 'Notify' }], edges: [] }, + try: { nodes: [{ id: 'notify', type: 'notify', label: 'Notify', config: { title: 'Notice', recipients: ['user_1'] } }], edges: [] }, catch: { nodes: [{ id: 'seen', type: 'capture', label: 'Seen' }], edges: [] }, }, }, diff --git a/packages/services/service-automation/src/engine.test.ts b/packages/services/service-automation/src/engine.test.ts index d178c51e1f4..a633b1e73b0 100644 --- a/packages/services/service-automation/src/engine.test.ts +++ b/packages/services/service-automation/src/engine.test.ts @@ -250,7 +250,7 @@ describe('AutomationEngine', () => { ], nodes: [ { id: 'start', type: 'start', label: 'Start' }, - { id: 'run', type: 'script', label: 'Run' }, + { id: 'run', type: 'script', label: 'Run', config: { function: 'noop' } }, { id: 'end', type: 'end', label: 'End' }, ], edges: [ @@ -280,7 +280,7 @@ describe('AutomationEngine', () => { type: 'record_change', nodes: [ { id: 'start', type: 'start', label: 'Start' }, - { id: 'run', type: 'script', label: 'Run' }, + { id: 'run', type: 'script', label: 'Run', config: { function: 'noop' } }, { id: 'end', type: 'end', label: 'End' }, ], edges: [ @@ -304,7 +304,7 @@ describe('AutomationEngine', () => { type: 'autolaunched', nodes: [ { id: 'start', type: 'start', label: 'Start' }, - { id: 'unknown', type: 'get_record', label: 'Get' }, + { id: 'unknown', type: 'get_record', label: 'Get', config: { objectName: 'task' } }, { id: 'end', type: 'end', label: 'End' }, ], edges: [ @@ -348,7 +348,7 @@ describe('AutomationEngine', () => { type: 'autolaunched', nodes: [ { id: 'start', type: 'start', label: 'Start' }, - { id: 'fail', type: 'script', label: 'Fail' }, + { id: 'fail', type: 'script', label: 'Fail', config: { function: 'noop' } }, { id: 'end', type: 'end', label: 'End' }, ], edges: [ @@ -1526,8 +1526,8 @@ describe('AutomationEngine - Fault Edge Support', () => { variables: [{ name: 'status', type: 'text', isOutput: true }], nodes: [ { id: 'start', type: 'start', label: 'Start' }, - { id: 'risky', type: 'script', label: 'Risky' }, - { id: 'handler', type: 'script', label: 'Error Handler' }, + { id: 'risky', type: 'script', label: 'Risky', config: { function: 'noop' } }, + { id: 'handler', type: 'script', label: 'Error Handler', config: { function: 'noop' } }, { id: 'end', type: 'end', label: 'End' }, ], edges: [ @@ -1563,8 +1563,8 @@ describe('AutomationEngine - Fault Edge Support', () => { type: 'autolaunched', nodes: [ { id: 'start', type: 'start', label: 'Start' }, - { id: 'risky', type: 'script', label: 'Risky' }, - { id: 'handler', type: 'script', label: 'Handler' }, + { id: 'risky', type: 'script', label: 'Risky', config: { function: 'noop' } }, + { id: 'handler', type: 'script', label: 'Handler', config: { function: 'noop' } }, { id: 'end', type: 'end', label: 'End' }, ], edges: [ @@ -1594,7 +1594,7 @@ describe('AutomationEngine - Fault Edge Support', () => { type: 'autolaunched', nodes: [ { id: 'start', type: 'start', label: 'Start' }, - { id: 'fail', type: 'script', label: 'Fail' }, + { id: 'fail', type: 'script', label: 'Fail', config: { function: 'noop' } }, { id: 'end', type: 'end', label: 'End' }, ], edges: [ @@ -1668,7 +1668,7 @@ describe('AutomationEngine - Step-Level Execution Logs', () => { type: 'autolaunched', nodes: [ { id: 'start', type: 'start', label: 'Start' }, - { id: 'bad', type: 'script', label: 'Bad' }, + { id: 'bad', type: 'script', label: 'Bad', config: { function: 'noop' } }, { id: 'end', type: 'end', label: 'End' }, ], edges: [ @@ -1937,7 +1937,7 @@ describe('AutomationEngine - Node Timeout', () => { type: 'autolaunched', nodes: [ { id: 'start', type: 'start', label: 'Start' }, - { id: 'slow', type: 'script', label: 'Slow', timeoutMs: 50 }, + { id: 'slow', type: 'script', label: 'Slow', timeoutMs: 50, config: { function: 'noop' } }, { id: 'end', type: 'end', label: 'End' }, ], edges: [ @@ -1965,7 +1965,7 @@ describe('AutomationEngine - Node Timeout', () => { type: 'autolaunched', nodes: [ { id: 'start', type: 'start', label: 'Start' }, - { id: 'fast', type: 'script', label: 'Fast', timeoutMs: 5000 }, + { id: 'fast', type: 'script', label: 'Fast', timeoutMs: 5000, config: { function: 'noop' } }, { id: 'end', type: 'end', label: 'End' }, ], edges: [ @@ -2012,7 +2012,7 @@ describe('AutomationEngine - Node Timeout', () => { id: `n${i}`, type: 'script', label: `Guarded ${i}`, - timeoutMs: GUARD_MS, + timeoutMs: GUARD_MS, config: { function: 'noop' }, })), { id: 'end', type: 'end', label: 'End' }, ]; @@ -2094,7 +2094,7 @@ describe('AutomationEngine - Node Timeout', () => { type: 'autolaunched', nodes: [ { id: 'start', type: 'start', label: 'Start' }, - { id: 'hangs', type: 'script', label: 'Hangs', timeoutMs: 50 }, + { id: 'hangs', type: 'script', label: 'Hangs', timeoutMs: 50, config: { function: 'noop' } }, { id: 'end', type: 'end', label: 'End' }, ], edges: [ @@ -2346,8 +2346,8 @@ describe('AutomationEngine - Parallel Branch Execution', () => { type: 'autolaunched', nodes: [ { id: 'start', type: 'start', label: 'Start' }, - { id: 'branch_a', type: 'script', label: 'Branch A', config: { delay: 10 } }, - { id: 'branch_b', type: 'script', label: 'Branch B', config: { delay: 10 } }, + { id: 'branch_a', type: 'script', label: 'Branch A', config: { function: 'noop', delay: 10 } }, + { id: 'branch_b', type: 'script', label: 'Branch B', config: { function: 'noop', delay: 10 } }, { id: 'end', type: 'end', label: 'End' }, ], edges: [ @@ -2414,7 +2414,7 @@ describe('AutomationEngine - Node Input Schema Validation', () => { id: 'validated', type: 'script', label: 'Validated', - config: {}, + config: { function: 'noop' }, inputSchema: { url: { type: 'string', required: true, description: 'URL to call' }, }, @@ -2450,7 +2450,7 @@ describe('AutomationEngine - Node Input Schema Validation', () => { id: 'validated', type: 'script', label: 'Validated', - config: { count: 'not_a_number' }, + config: { function: 'noop', count: 'not_a_number' }, inputSchema: { count: { type: 'number', required: true }, }, @@ -2563,7 +2563,7 @@ describe('AutomationEngine - Execution Status', () => { type: 'autolaunched', nodes: [ { id: 'start', type: 'start', label: 'Start' }, - { id: 'bad', type: 'script', label: 'Bad' }, + { id: 'bad', type: 'script', label: 'Bad', config: { function: 'noop' } }, { id: 'end', type: 'end', label: 'End' }, ], edges: [ @@ -3237,7 +3237,7 @@ describe('#9378 — execute() classifies terminal exits for the trigger transpor // from the trigger door and is not dead code. engine.registerFlow('startless', { name: 'startless', label: 'Startless', type: 'autolaunched', - nodes: [{ id: 'middle', type: 'script', label: 'Middle' }], + nodes: [{ id: 'middle', type: 'script', label: 'Middle', config: { function: 'noop' } }], edges: [], }); const startless = await engine.execute('startless'); @@ -3271,7 +3271,7 @@ describe('#9378 — execute() classifies terminal exits for the trigger transpor engine.registerFlow('startless_coded', { name: 'startless_coded', label: 'Startless', type: 'autolaunched', - nodes: [{ id: 'middle', type: 'script', label: 'Middle' }], + nodes: [{ id: 'middle', type: 'script', label: 'Middle', config: { function: 'noop' } }], edges: [], }); const startless = await engine.execute('startless_coded'); diff --git a/packages/services/service-automation/src/flow-terminal-messages.test.ts b/packages/services/service-automation/src/flow-terminal-messages.test.ts index c20c5b4c83e..c6e480dd57d 100644 --- a/packages/services/service-automation/src/flow-terminal-messages.test.ts +++ b/packages/services/service-automation/src/flow-terminal-messages.test.ts @@ -85,7 +85,7 @@ function messageFlow( label: 'Start', ...(opts.startCondition ? { config: { condition: opts.startCondition } } : {}), }, - { id: 'work', type: 'script', label: 'Work' }, + { id: 'work', type: 'script', label: 'Work', config: { function: 'noop' } }, { id: 'end', type: 'end', label: 'End' }, ], edges: [ diff --git a/packages/spec/src/api/zod-issues-to-fields.test.ts b/packages/spec/src/api/zod-issues-to-fields.test.ts index c6db86cb443..3db430cc9be 100644 --- a/packages/spec/src/api/zod-issues-to-fields.test.ts +++ b/packages/spec/src/api/zod-issues-to-fields.test.ts @@ -42,7 +42,9 @@ const WELL_FORMED_FLOW = { name: 'welcome_flow', label: 'Welcome', type: 'autolaunched', - nodes: [{ id: 'n', type: 'notify', label: 'Notify', config: { message: 'hi' } }], + // `recipients` and `title` are the keys the notify executor contract + // requires; the flow parse refuses a notify node that leaves them out. + nodes: [{ id: 'n', type: 'notify', label: 'Notify', config: { recipients: ['{record.owner}'], title: 'Welcome', message: 'hi' } }], edges: [], }; diff --git a/packages/spec/src/automation/flow-node-config-refusals.ts b/packages/spec/src/automation/flow-node-config-refusals.ts new file mode 100644 index 00000000000..63a26c991d6 --- /dev/null +++ b/packages/spec/src/automation/flow-node-config-refusals.ts @@ -0,0 +1,325 @@ +// Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license. + +/** + * @module automation/flow-node-config-refusals + * + * **What a node's executor needs its `config` to carry** (#20316) — the one + * judge `FlowSchema.parse`, `AutomationEngine.registerFlow` (which parses + * first) and `objectstack validate` share, beside the expression ledger's + * `predicateSlotRefusal` and closing the same gap: a node's `config` is an + * open `z.record`, so what its executor requires was checked by nobody until + * the run. + * + * Its refusal codes join the closed flow slot table + * (`FLOW_SLOT_REFUSAL_CODES`, `flow-node-expression-paths.ts`); the + * judge lives in this module rather than that one because it reads the + * executor contracts, and two of their modules import that one — a leaf it + * has to stay. + */ + +import { NON_BLANK_STRING } from '../shared/refinement-projection'; +import { FLOW_REGION_SLOTS_BY_TYPE } from './region-slots'; +import type { FlowNodeConfigRefusal, FlowSlotRefusalParams, NodeConfigValueKind } from './flow-node-expression-paths'; +// The executor contracts. Read only inside `getBuiltinNodeConfigContracts`, +// never at module load: `control-flow.zod.ts` sits in the flow-schema import +// cycle, so its bindings are live references resolved on first use. +import { LoopConfigSchema, ParallelConfigSchema, TryCatchConfigSchema } from './control-flow.zod'; +import { + CreateRecordConfigSchema, + DeleteRecordConfigSchema, + GetRecordConfigSchema, + MapConfigSchema, + ScreenConfigSchema, + UpdateRecordConfigSchema, +} from './builtin-node-config.zod'; +import { HttpConfigSchema, NotifyConfigSchema } from './io-node-config.zod'; +import { ScriptConfigSchema, SubflowConfigSchema } from './schemaless-node-config.zod'; + +/** + * The executor contract a builtin node's `config` is parsed against at run + * time — the SAME Zod schema its executor hands `parseNodeConfig` + * (`service-automation/builtin/parse-config.ts`) — and, where the executor + * parses only on one path, the condition it parses on. + * + * Structural, like `parseNodeConfig`'s own view of a contract: this module + * reads `safeParse` and nothing else. + */ +export interface BuiltinNodeConfigContract { + readonly schema: { + safeParse(value: unknown): { + success: boolean; + error?: { issues: ReadonlyArray<{ code: string; path: ReadonlyArray; message: string }> }; + }; + }; + /** + * The executor parses the config only when this holds. Absent: always. The + * one member is `loop`, whose legacy flat-graph form (no `body`) predates + * the ADR-0031 construct its contract describes and is deliberately not + * parsed (`loop-node.ts`), so `collection` is required only once a `body` + * is there. + */ + readonly parsedWhen?: (config: Readonly>) => boolean; +} + +let cachedBuiltinNodeConfigContracts: ReadonlyMap | undefined; + +/** + * Every builtin node type whose executor parses its `config` against a + * contract at run time, keyed by `node.type` (#20316). + * + * The declared half of a pair: `service-automation`'s ratchet + * (`node-config-contract-ledger.test.ts`) reads each executor's + * `parseNodeConfig(…)` call out of its source and holds this map equal to it + * in both directions — the type, the schema, and `loop`'s parse condition — + * so a new contract-parsing executor cannot go unjudged here, and an entry + * cannot outlive the parse it mirrors. + * + * Built on first use, never at module load: these schemas' modules import + * this one, and a map literal at top level would read them mid-cycle. + * + * NOT here, on purpose: `decision` (its executor parses nothing — its branch + * shape is judged by {@link flowNodeConfigRefusals}'s own arm), `assignment` + * (three read-compatible shapes, no single contract), and `wait` / + * `connector_action`, whose inputs are FlowNode SIBLING blocks + * (`waitEventConfig` / `connectorConfig`), not `config`. + */ +export function getBuiltinNodeConfigContracts(): ReadonlyMap { + if (cachedBuiltinNodeConfigContracts === undefined) { + cachedBuiltinNodeConfigContracts = new Map([ + ['get_record', { schema: GetRecordConfigSchema }], + ['create_record', { schema: CreateRecordConfigSchema }], + ['update_record', { schema: UpdateRecordConfigSchema }], + ['delete_record', { schema: DeleteRecordConfigSchema }], + ['notify', { schema: NotifyConfigSchema }], + ['http', { schema: HttpConfigSchema }], + ['screen', { schema: ScreenConfigSchema }], + ['script', { schema: ScriptConfigSchema }], + ['subflow', { schema: SubflowConfigSchema }], + ['map', { schema: MapConfigSchema }], + ['loop', { schema: LoopConfigSchema, parsedWhen: (config) => config.body != null }], + ['parallel', { schema: ParallelConfigSchema }], + ['try_catch', { schema: TryCatchConfigSchema }], + ]); + } + return cachedBuiltinNodeConfigContracts; +} + +/** A plain object (not an array, not `null`). */ +function isRecord(value: unknown): value is Record { + return typeof value === 'object' && value !== null && !Array.isArray(value); +} + +/** The kind of a value that is not the shape a node-config position wants. */ +function nodeConfigValueKind(value: unknown): NodeConfigValueKind { + if (Array.isArray(value)) return 'array'; + return typeof value as NodeConfigValueKind; +} + +/** `a string`, `an array`, `an object` — the phrase a kind token renders as. */ +function kindPhrase(kind: NodeConfigValueKind | 'null'): string { + if (kind === 'null') return '`null`'; + return kind === 'array' || kind === 'object' ? `an ${kind}` : `a ${kind}`; +} + +/** A Zod issue path → the ledger spelling (`['fields', 0, 'name']` → `fields[0].name`). */ +function ledgerPathOf(path: ReadonlyArray): string { + let out = ''; + for (const segment of path) { + if (typeof segment === 'number') out += `[${segment}]`; + else out += out ? `.${String(segment)}` : String(segment); + } + return out; +} + +/** + * Is the key an issue names ABSENT from the authored config — its parent + * reached, and the key itself not there (or `undefined`)? The one question + * the contract arm asks: a present value of the wrong type is a different + * finding, and not this judge's. + */ +function absentAt(config: Readonly>, path: ReadonlyArray): boolean { + let parent: unknown = config; + for (const segment of path.slice(0, -1)) { + if (parent === null || typeof parent !== 'object') return false; + parent = (parent as Record)[segment as PropertyKey]; + } + if (parent === null || typeof parent !== 'object') return false; + const last = path[path.length - 1] as PropertyKey; + return (parent as Record)[last] === undefined; +} + +/** + * Does an issue path descend INTO an ADR-0031 region (`body.nodes…`, + * `branches[0]…`, `try.edges…`)? Those are the region's own nodes and edges, + * judged where the walks reach them as a graph — never re-reported against + * the container that holds them. The slot itself absent (`try`, `branches`) + * is the container's, and is judged here. + */ +function insideRegion(nodeType: string, path: ReadonlyArray): boolean { + if (path.length < 2) return false; + return (FLOW_REGION_SLOTS_BY_TYPE.get(nodeType) ?? []).some((slot) => slot.key === path[0]); +} + +/** The refusal for a key a node's executor contract requires. */ +function nodeConfigKeyMissingMessage(nodeType: string, key: string): string { + return ( + `This \`${nodeType}\` node's config leaves out \`${key}\`, which the ${nodeType} contract requires. Its executor ` + + 'parses the config against that contract before it does anything else and refuses the node without it — so ' + + 'the flow registers, and then every run that reaches this node fails there; the config is metadata, and ' + + `re-running changes nothing. Write \`${key}\` on the node's \`config\`.` + ); +} + +/** + * Every reason a node's `config` is refused on SHAPE or PRESENCE — the ONE + * judge `FlowSchema.parse`, `AutomationEngine.registerFlow` (which parses + * first) and `objectstack validate` share (#20316). + * + * Two arms. + * + * ## The executor contract — a key it requires, absent + * + * For a type in {@link getBuiltinNodeConfigContracts}, the config is parsed + * against the executor's own contract, on the executor's own condition + * (`config ?? {}`, as `parseNodeConfig` reads it; `loop` only with a `body`), + * and a failure is kept ONLY where the key it names is absent from what was + * authored. That keeps the judge to one question — "would the run refuse this + * node for a key it leaves out?" — and leaves every other contract finding + * (a present value of the wrong type, an undeclared key) where it lives + * today. Issues inside an ADR-0031 region are the region's own and skipped. + * + * - A key the contract simply requires → `node-config-key-missing`, whose + * message names the key and the node type. + * - A key a RULE of the contract requires in this configuration (a `notify` + * with no `template` needs `title`; a `lookup` screen field needs its + * `reference`) → `node-config-key-required-by-rule`, whose message is the + * contract's own. + * + * A key only the conversion layer spells canonically (`object` → + * `objectName`, `flow` → `flowName`, …) is judged AFTER the conversion at + * `registerFlow` and `objectstack validate`, which convert first; a direct + * `FlowSchema.parse` of a pre-conversion spelling meets the refusal, exactly + * as it meets every other tombstone. + * + * ## The decision branch shape + * + * `decision` is parsed by nothing at run time — its executor reads + * `conditions[]` raw — so its arm states what that read needs: + * + * - `conditions` present and not `null` is an array + * (`decision-conditions-not-array`) — the executor iterates it; + * - every element is an object (`decision-branch-not-object`) — the + * executor reads `label` and `expression` off it; + * - every branch's `label` is a non-blank string + * (`decision-branch-label-missing`) — the label is the branch: the matched + * branch reports it and traversal keeps only the out-edge carrying it, so + * a branch without one selects nothing and EVERY out-edge is considered. + * + * The branch's `expression` is not judged here: it is a ledger `predicate` + * slot, refused absent / blank / non-text by `predicateSlotRefusal`. + * + * Every refusal carries `source: ''`: none of these values is CEL text. + */ +export function flowNodeConfigRefusals(nodeType: string, config: unknown): FlowNodeConfigRefusal[] { + const out: FlowNodeConfigRefusal[] = []; + if (nodeType === 'decision') { + decisionShapeRefusals(config, out); + return out; + } + const contract = getBuiltinNodeConfigContracts().get(nodeType); + if (!contract) return out; + const authored = config ?? {}; + if (!isRecord(authored)) return out; + if (contract.parsedWhen && !contract.parsedWhen(authored)) return out; + const result = contract.schema.safeParse(authored); + if (result.success) return out; + const seen = new Set(); + for (const issue of result.error?.issues ?? []) { + if (issue.path.length === 0) continue; + if (insideRegion(nodeType, issue.path)) continue; + if (!absentAt(authored, issue.path)) continue; + const key = ledgerPathOf(issue.path); + if (seen.has(key)) continue; + seen.add(key); + out.push( + issue.code === 'custom' + ? { code: 'node-config-key-required-by-rule', params: { nodeType, key }, message: issue.message, source: '', path: key } + : { + code: 'node-config-key-missing', + params: { nodeType, key }, + message: nodeConfigKeyMissingMessage(nodeType, key), + source: '', + path: key, + }, + ); + } + return out; +} + +/** The `decision` arm of {@link flowNodeConfigRefusals}. */ +function decisionShapeRefusals(config: unknown, out: FlowNodeConfigRefusal[]): void { + if (!isRecord(config)) return; + const conditions = config.conditions; + // Absent or `null` declares no branch: the executor reads `?? []` and the + // node routes by its out-edges, which is legal. + if (conditions == null) return; + if (!Array.isArray(conditions)) { + const found = nodeConfigValueKind(conditions) as Exclude; + out.push({ + code: 'decision-conditions-not-array', + params: { found }, + message: + `A decision's \`conditions\` is its ordered branch list — an array of \`{ label, expression }\` — and this ` + + `one is ${kindPhrase(found)}. The decision executor iterates it, so a run that reaches the node fails ` + + 'there (a string is iterated character by character, each character a branch with no `expression`). ' + + 'Write the branches as an array, or delete `conditions` and route by the out-edges\' own `condition`s.', + source: '', + path: 'conditions', + }); + return; + } + conditions.forEach((branch: unknown, index: number) => { + const path = `conditions[${index}]`; + if (!isRecord(branch)) { + const found = branch === null ? 'null' : (nodeConfigValueKind(branch) as Exclude); + out.push({ + code: 'decision-branch-not-object', + params: { index, found }, + message: + `A decision branch is an object — \`{ label, expression }\` — and \`${path}\` is ${kindPhrase(found)}. ` + + 'The decision executor reads `label` and `expression` off every branch it reaches, so this one has ' + + 'neither and a run that reaches it fails at the node. Write it as `{ label: \'approved\', expression: ' + + '\'record.amount > 1000\' }` — the label of the out-edge it routes to, and a bare CEL predicate; a ' + + 'predicate written as a bare string belongs under `expression`.', + source: '', + path, + }); + return; + } + const label = branch.label; + if (typeof label === 'string' && NON_BLANK_STRING(label)) return; + const found: FlowSlotRefusalParams['decision-branch-label-missing']['found'] = + label === undefined ? 'absent' + : label === null ? 'null' + : typeof label === 'string' ? 'blank' + : (nodeConfigValueKind(label) as Exclude); + const phrase = + found === 'absent' ? 'nothing — the key is absent' + : found === 'blank' ? 'a string that is blank after trimming' + : kindPhrase(found); + out.push({ + code: 'decision-branch-label-missing', + params: { index, found }, + message: + 'A decision branch routes by its `label`: the first branch whose `expression` holds is taken, and the run ' + + `continues down the out-edge carrying that label. \`${path}.label\` holds ${phrase}, and that names no ` + + 'out-edge — so when this branch matches, the node reports no branch it can route, and traversal ' + + 'considers EVERY out-edge instead, as if the decision declared no branches: an unconditional labelled ' + + 'out-edge and the default out-edge both run. Write the label of the out-edge this branch should take ' + + '(`label: \'approved\'` for the out-edge labelled `approved`). To branch on the out-edges instead, ' + + 'delete `conditions` and put each predicate on its edge\'s `condition`.', + source: '', + path: `${path}.label`, + }); + }); +} diff --git a/packages/spec/src/automation/flow-node-config-required.test.ts b/packages/spec/src/automation/flow-node-config-required.test.ts index 178759f21b4..5c10479333a 100644 --- a/packages/spec/src/automation/flow-node-config-required.test.ts +++ b/packages/spec/src/automation/flow-node-config-required.test.ts @@ -29,7 +29,7 @@ import { describe, expect, it } from 'vitest'; import { NotifyConfigSchema } from './io-node-config.zod'; import { ScreenConfigSchema } from './builtin-node-config.zod'; -import { flowNodeConfigRefusals } from './flow-node-expression-paths'; +import { flowNodeConfigRefusals } from './flow-node-config-refusals'; import { FlowSchema } from './flow.zod'; type Node = Record; diff --git a/packages/spec/src/automation/flow-node-expression-paths.ts b/packages/spec/src/automation/flow-node-expression-paths.ts index b3bdef23a2f..7d353fd1fc3 100644 --- a/packages/spec/src/automation/flow-node-expression-paths.ts +++ b/packages/spec/src/automation/flow-node-expression-paths.ts @@ -75,21 +75,6 @@ // The repo's one notion of "blank" (`source.trim()`), shared with the evaluated // slots — never a second hand-written one here. import { NON_BLANK_STRING } from '../shared/refinement-projection'; -import { FLOW_REGION_SLOTS_BY_TYPE } from './region-slots'; -// The executor contracts (#20316). Read only inside -// `getBuiltinNodeConfigContracts`, never at module load: two of these modules -// import this one, so the bindings are live references resolved on first use. -import { LoopConfigSchema, ParallelConfigSchema, TryCatchConfigSchema } from './control-flow.zod'; -import { - CreateRecordConfigSchema, - DeleteRecordConfigSchema, - GetRecordConfigSchema, - MapConfigSchema, - ScreenConfigSchema, - UpdateRecordConfigSchema, -} from './builtin-node-config.zod'; -import { HttpConfigSchema, NotifyConfigSchema } from './io-node-config.zod'; -import { ScriptConfigSchema, SubflowConfigSchema } from './schemaless-node-config.zod'; /** * The dialect a declared expression slot takes — and therefore what, if @@ -184,7 +169,8 @@ export interface FlowNodeExpressionPath { * this flag cannot claim a requirement the contract does not make, nor miss * one it does. Never set on another role: the channels require * `loop.collection` / `map.collection` too, and since #20316 all three doors - * refuse their absence — but through {@link flowNodeConfigRefusals}, which + * refuse their absence — but through `flowNodeConfigRefusals` + * (`flow-node-config-refusals.ts`), which * judges every key an executor contract requires, not through this flag, * which only decides what the expression walk emits. */ @@ -372,7 +358,7 @@ export function isExpressionEnvelopeShaped(value: unknown): value is { dialect: * Only the element's OWN slot is judged: an element that is not an object * carries no slot, and the walk does not reach it — an ARRAY element * included, since #20316 (the walk used to read an array element as an - * object missing its slot); {@link flowNodeConfigRefusals} refuses such an + * object missing its slot); `flowNodeConfigRefusals` refuses such an * element as what it is. A **non-string** * is emitted too (#15572), for the consumer to refuse through * {@link predicateSlotRefusal}: it used to be skipped as "a type violation @@ -504,7 +490,8 @@ export type StructuralConditionValueKind = /** * What a value sitting where a node's `config` wants another shape was, as a - * token — for {@link flowNodeConfigRefusals}. `null` is spelled out on the + * token — for `flowNodeConfigRefusals` (`flow-node-config-refusals.ts`). + * `null` is spelled out on the * codes that can meet it; the message renders the token as a phrase (`a * string`, `an array`, `an object`). */ @@ -519,10 +506,12 @@ export type NodeConfigValueKind = | 'object'; /** - * Refusal code → the params its message interpolates, for this file's three - * refusal producers, {@link predicateSlotRefusal}, - * {@link structuralConditionRefusal} and {@link flowNodeConfigRefusals}. The - * keys ARE the closed set. + * Refusal code → the params its message interpolates, for the three flow slot + * refusal producers — this file's {@link predicateSlotRefusal} and + * {@link structuralConditionRefusal}, and (#20316) `flowNodeConfigRefusals` + * in `flow-node-config-refusals.ts`, kept in its own module because it reads + * the executor contracts, whose modules import this one. The keys ARE the + * closed set, for all three. * * A consumer that renders its own words — a localized designer — keys its * catalogue row to the `code` and fills it from the `params`; the English @@ -559,7 +548,7 @@ export interface FlowSlotRefusalParams { 'node-config-key-required-by-rule': { readonly nodeType: string; readonly key: string }; } -/** Every refusal code this file's producers emit. */ +/** Every refusal code the three flow slot refusal producers emit. */ export type FlowSlotRefusalCode = keyof FlowSlotRefusalParams; /** The codes {@link predicateSlotRefusal} emits. */ @@ -568,7 +557,7 @@ export type PredicateSlotRefusalCode = 'predicate-slot-missing' | 'predicate-slo /** The codes {@link structuralConditionRefusal} emits. */ export type StructuralConditionRefusalCode = 'structural-condition-shape'; -/** The codes {@link flowNodeConfigRefusals} emits. */ +/** The codes `flowNodeConfigRefusals` (`flow-node-config-refusals.ts`) emits. */ export type FlowNodeConfigRefusalCode = | 'decision-conditions-not-array' | 'decision-branch-not-object' @@ -621,7 +610,7 @@ const FLOW_SLOT_REFUSAL_CODE_TABLE = { } as const satisfies Record; /** - * The closed set of this file's refusal codes, as a value — for a consumer + * The closed set of the flow slot refusal codes, as a value — for a consumer * that must prove it has a catalogue row for every code. */ export const FLOW_SLOT_REFUSAL_CODES: readonly FlowSlotRefusalCode[] = Object.freeze( @@ -942,300 +931,6 @@ export function structuralConditionRefusal( }; } -// ─── Node config the executor requires (#20316) ───────────────────── - -/** - * The executor contract a builtin node's `config` is parsed against at run - * time — the SAME Zod schema its executor hands `parseNodeConfig` - * (`service-automation/builtin/parse-config.ts`) — and, where the executor - * parses only on one path, the condition it parses on. - * - * Structural, like `parseNodeConfig`'s own view of a contract: this module - * reads `safeParse` and nothing else. - */ -export interface BuiltinNodeConfigContract { - readonly schema: { - safeParse(value: unknown): { - success: boolean; - error?: { issues: ReadonlyArray<{ code: string; path: ReadonlyArray; message: string }> }; - }; - }; - /** - * The executor parses the config only when this holds. Absent: always. The - * one member is `loop`, whose legacy flat-graph form (no `body`) predates - * the ADR-0031 construct its contract describes and is deliberately not - * parsed (`loop-node.ts`), so `collection` is required only once a `body` - * is there. - */ - readonly parsedWhen?: (config: Readonly>) => boolean; -} - -let cachedBuiltinNodeConfigContracts: ReadonlyMap | undefined; - -/** - * Every builtin node type whose executor parses its `config` against a - * contract at run time, keyed by `node.type` (#20316). - * - * The declared half of a pair: `service-automation`'s ratchet - * (`node-config-contract-ledger.test.ts`) reads each executor's - * `parseNodeConfig(…)` call out of its source and holds this map equal to it - * in both directions — the type, the schema, and `loop`'s parse condition — - * so a new contract-parsing executor cannot go unjudged here, and an entry - * cannot outlive the parse it mirrors. - * - * Built on first use, never at module load: these schemas' modules import - * this one, and a map literal at top level would read them mid-cycle. - * - * NOT here, on purpose: `decision` (its executor parses nothing — its branch - * shape is judged by {@link flowNodeConfigRefusals}'s own arm), `assignment` - * (three read-compatible shapes, no single contract), and `wait` / - * `connector_action`, whose inputs are FlowNode SIBLING blocks - * (`waitEventConfig` / `connectorConfig`), not `config`. - */ -export function getBuiltinNodeConfigContracts(): ReadonlyMap { - if (cachedBuiltinNodeConfigContracts === undefined) { - cachedBuiltinNodeConfigContracts = new Map([ - ['get_record', { schema: GetRecordConfigSchema }], - ['create_record', { schema: CreateRecordConfigSchema }], - ['update_record', { schema: UpdateRecordConfigSchema }], - ['delete_record', { schema: DeleteRecordConfigSchema }], - ['notify', { schema: NotifyConfigSchema }], - ['http', { schema: HttpConfigSchema }], - ['screen', { schema: ScreenConfigSchema }], - ['script', { schema: ScriptConfigSchema }], - ['subflow', { schema: SubflowConfigSchema }], - ['map', { schema: MapConfigSchema }], - ['loop', { schema: LoopConfigSchema, parsedWhen: (config) => config.body != null }], - ['parallel', { schema: ParallelConfigSchema }], - ['try_catch', { schema: TryCatchConfigSchema }], - ]); - } - return cachedBuiltinNodeConfigContracts; -} - -/** A plain object (not an array, not `null`). */ -function isRecord(value: unknown): value is Record { - return typeof value === 'object' && value !== null && !Array.isArray(value); -} - -/** The kind of a value that is not the shape a node-config position wants. */ -function nodeConfigValueKind(value: unknown): NodeConfigValueKind { - if (Array.isArray(value)) return 'array'; - return typeof value as NodeConfigValueKind; -} - -/** `a string`, `an array`, `an object` — the phrase a kind token renders as. */ -function kindPhrase(kind: NodeConfigValueKind | 'null'): string { - if (kind === 'null') return '`null`'; - return kind === 'array' || kind === 'object' ? `an ${kind}` : `a ${kind}`; -} - -/** A Zod issue path → the ledger spelling (`['fields', 0, 'name']` → `fields[0].name`). */ -function ledgerPathOf(path: ReadonlyArray): string { - let out = ''; - for (const segment of path) { - if (typeof segment === 'number') out += `[${segment}]`; - else out += out ? `.${String(segment)}` : String(segment); - } - return out; -} - -/** - * Is the key an issue names ABSENT from the authored config — its parent - * reached, and the key itself not there (or `undefined`)? The one question - * the contract arm asks: a present value of the wrong type is a different - * finding, and not this judge's. - */ -function absentAt(config: Readonly>, path: ReadonlyArray): boolean { - let parent: unknown = config; - for (const segment of path.slice(0, -1)) { - if (parent === null || typeof parent !== 'object') return false; - parent = (parent as Record)[segment as PropertyKey]; - } - if (parent === null || typeof parent !== 'object') return false; - const last = path[path.length - 1] as PropertyKey; - return (parent as Record)[last] === undefined; -} - -/** - * Does an issue path descend INTO an ADR-0031 region (`body.nodes…`, - * `branches[0]…`, `try.edges…`)? Those are the region's own nodes and edges, - * judged where the walks reach them as a graph — never re-reported against - * the container that holds them. The slot itself absent (`try`, `branches`) - * is the container's, and is judged here. - */ -function insideRegion(nodeType: string, path: ReadonlyArray): boolean { - if (path.length < 2) return false; - return (FLOW_REGION_SLOTS_BY_TYPE.get(nodeType) ?? []).some((slot) => slot.key === path[0]); -} - -/** The refusal for a key a node's executor contract requires. */ -function nodeConfigKeyMissingMessage(nodeType: string, key: string): string { - return ( - `This \`${nodeType}\` node's config leaves out \`${key}\`, which the ${nodeType} contract requires. Its executor ` - + 'parses the config against that contract before it does anything else and refuses the node without it — so ' - + 'the flow registers, and then every run that reaches this node fails there; the config is metadata, and ' - + `re-running changes nothing. Write \`${key}\` on the node's \`config\`.` - ); -} - -/** - * Every reason a node's `config` is refused on SHAPE or PRESENCE — the ONE - * judge `FlowSchema.parse`, `AutomationEngine.registerFlow` (which parses - * first) and `objectstack validate` share (#20316), in this module beside - * {@link predicateSlotRefusal} because it closes the same gap: a node's - * `config` is an open `z.record`, so what its executor requires was checked - * by nobody until the run. - * - * Two arms. - * - * ## The executor contract — a key it requires, absent - * - * For a type in {@link getBuiltinNodeConfigContracts}, the config is parsed - * against the executor's own contract, on the executor's own condition - * (`config ?? {}`, as `parseNodeConfig` reads it; `loop` only with a `body`), - * and a failure is kept ONLY where the key it names is absent from what was - * authored. That keeps the judge to one question — "would the run refuse this - * node for a key it leaves out?" — and leaves every other contract finding - * (a present value of the wrong type, an undeclared key) where it lives - * today. Issues inside an ADR-0031 region are the region's own and skipped. - * - * - A key the contract simply requires → `node-config-key-missing`, whose - * message names the key and the node type. - * - A key a RULE of the contract requires in this configuration (a `notify` - * with no `template` needs `title`; a `lookup` screen field needs its - * `reference`) → `node-config-key-required-by-rule`, whose message is the - * contract's own. - * - * A key only the conversion layer spells canonically (`object` → - * `objectName`, `flow` → `flowName`, …) is judged AFTER the conversion at - * `registerFlow` and `objectstack validate`, which convert first; a direct - * `FlowSchema.parse` of a pre-conversion spelling meets the refusal, exactly - * as it meets every other tombstone. - * - * ## The decision branch shape - * - * `decision` is parsed by nothing at run time — its executor reads - * `conditions[]` raw — so its arm states what that read needs: - * - * - `conditions` present and not `null` is an array - * (`decision-conditions-not-array`) — the executor iterates it; - * - every element is an object (`decision-branch-not-object`) — the - * executor reads `label` and `expression` off it; - * - every branch's `label` is a non-blank string - * (`decision-branch-label-missing`) — the label is the branch: the matched - * branch reports it and traversal keeps only the out-edge carrying it, so - * a branch without one selects nothing and EVERY out-edge is considered. - * - * The branch's `expression` is not judged here: it is a ledger `predicate` - * slot, refused absent / blank / non-text by {@link predicateSlotRefusal}. - * - * Every refusal carries `source: ''`: none of these values is CEL text. - */ -export function flowNodeConfigRefusals(nodeType: string, config: unknown): FlowNodeConfigRefusal[] { - const out: FlowNodeConfigRefusal[] = []; - if (nodeType === 'decision') { - decisionShapeRefusals(config, out); - return out; - } - const contract = getBuiltinNodeConfigContracts().get(nodeType); - if (!contract) return out; - const authored = config ?? {}; - if (!isRecord(authored)) return out; - if (contract.parsedWhen && !contract.parsedWhen(authored)) return out; - const result = contract.schema.safeParse(authored); - if (result.success) return out; - const seen = new Set(); - for (const issue of result.error?.issues ?? []) { - if (issue.path.length === 0) continue; - if (insideRegion(nodeType, issue.path)) continue; - if (!absentAt(authored, issue.path)) continue; - const key = ledgerPathOf(issue.path); - if (seen.has(key)) continue; - seen.add(key); - out.push( - issue.code === 'custom' - ? { code: 'node-config-key-required-by-rule', params: { nodeType, key }, message: issue.message, source: '', path: key } - : { - code: 'node-config-key-missing', - params: { nodeType, key }, - message: nodeConfigKeyMissingMessage(nodeType, key), - source: '', - path: key, - }, - ); - } - return out; -} - -/** The `decision` arm of {@link flowNodeConfigRefusals}. */ -function decisionShapeRefusals(config: unknown, out: FlowNodeConfigRefusal[]): void { - if (!isRecord(config)) return; - const conditions = config.conditions; - // Absent or `null` declares no branch: the executor reads `?? []` and the - // node routes by its out-edges, which is legal. - if (conditions == null) return; - if (!Array.isArray(conditions)) { - const found = nodeConfigValueKind(conditions) as Exclude; - out.push({ - code: 'decision-conditions-not-array', - params: { found }, - message: - `A decision's \`conditions\` is its ordered branch list — an array of \`{ label, expression }\` — and this ` - + `one is ${kindPhrase(found)}. The decision executor iterates it, so a run that reaches the node fails ` - + 'there (a string is iterated character by character, each character a branch with no `expression`). ' - + 'Write the branches as an array, or delete `conditions` and route by the out-edges\' own `condition`s.', - source: '', - path: 'conditions', - }); - return; - } - conditions.forEach((branch: unknown, index: number) => { - const path = `conditions[${index}]`; - if (!isRecord(branch)) { - const found = branch === null ? 'null' : (nodeConfigValueKind(branch) as Exclude); - out.push({ - code: 'decision-branch-not-object', - params: { index, found }, - message: - `A decision branch is an object — \`{ label, expression }\` — and \`${path}\` is ${kindPhrase(found)}. ` - + 'The decision executor reads `label` and `expression` off every branch it reaches, so this one has ' - + 'neither and a run that reaches it fails at the node. Write it as `{ label: \'approved\', expression: ' - + '\'record.amount > 1000\' }` — the label of the out-edge it routes to, and a bare CEL predicate; a ' - + 'predicate written as a bare string belongs under `expression`.', - source: '', - path, - }); - return; - } - const label = branch.label; - if (typeof label === 'string' && NON_BLANK_STRING(label)) return; - const found: FlowSlotRefusalParams['decision-branch-label-missing']['found'] = - label === undefined ? 'absent' - : label === null ? 'null' - : typeof label === 'string' ? 'blank' - : (nodeConfigValueKind(label) as Exclude); - const phrase = - found === 'absent' ? 'nothing — the key is absent' - : found === 'blank' ? 'a string that is blank after trimming' - : kindPhrase(found); - out.push({ - code: 'decision-branch-label-missing', - params: { index, found }, - message: - 'A decision branch routes by its `label`: the first branch whose `expression` holds is taken, and the run ' - + `continues down the out-edge carrying that label. \`${path}.label\` holds ${phrase}, and that names no ` - + 'out-edge — so when this branch matches, the node reports no branch it can route, and traversal ' - + 'considers EVERY out-edge instead, as if the decision declared no branches: an unconditional labelled ' - + 'out-edge and the default out-edge both run. Write the label of the out-edge this branch should take ' - + '(`label: \'approved\'` for the out-edge labelled `approved`). To branch on the out-edges instead, ' - + 'delete `conditions` and put each predicate on its edge\'s `condition`.', - source: '', - path: `${path}.label`, - }); - }); -} - /** * Descend `segments` through `node`, expanding a `key[]` segment over every * element of that array and a `*` segment over every own key of that object, diff --git a/packages/spec/src/automation/flow-region-pause-and-end.test.ts b/packages/spec/src/automation/flow-region-pause-and-end.test.ts index 8ed2314ff5e..9de9d33d9a2 100644 --- a/packages/spec/src/automation/flow-region-pause-and-end.test.ts +++ b/packages/spec/src/automation/flow-region-pause-and-end.test.ts @@ -242,10 +242,19 @@ describe('the rule does NOT over-reach', () => { }); it('leaves every non-pausing node type alone inside a region — the rule is a list, not a mood', () => { + // Each node carries the config its executor contract requires, so the parse + // judges only this rule — a config left out is refused by its own (#20316). + const WHOLE: Record> = { + create_record: { objectName: 'task' }, + update_record: { objectName: 'task', filter: { id: '{row.id}' } }, + get_record: { objectName: 'task' }, + http: { url: 'https://example.com/hook' }, + notify: { recipients: ['{row.owner}'], title: 'Row processed' }, + }; for (const type of ['assignment', 'decision', 'create_record', 'update_record', 'get_record', 'http', 'notify', 'loop']) { const node: FlowNode = type === 'loop' ? loopOver([step('inner')], 'inner_loop') - : { id: 'work', type, label: 'Work' }; + : { id: 'work', type, label: 'Work', ...(WHOLE[type] ? { config: WHOLE[type] } : {}) }; expect(FlowSchema.safeParse(flowWith([loopOver([node])])).success, type).toBe(true); } }); diff --git a/packages/spec/src/automation/flow-slot-refusal-codes.test.ts b/packages/spec/src/automation/flow-slot-refusal-codes.test.ts index 90e07f21cc1..3f1629a7291 100644 --- a/packages/spec/src/automation/flow-slot-refusal-codes.test.ts +++ b/packages/spec/src/automation/flow-slot-refusal-codes.test.ts @@ -25,7 +25,6 @@ import { FLOW_SLOT_REFUSAL_CODES, PREDICATE_SLOT_STRING_REFUSAL, STRUCTURAL_CONDITION_SHAPE_REFUSAL, - flowNodeConfigRefusals, predicateSlotRefusal, structuralConditionRefusal, type FlowNodeConfigRefusal, @@ -38,6 +37,7 @@ import { type StructuralConditionRefusalCode, } from './flow-node-expression-paths.js'; import * as automation from './index.js'; +import { flowNodeConfigRefusals } from './flow-node-config-refusals.js'; import { NotifyConfigSchema } from './io-node-config.zod.js'; /** What one refusal says, whichever producer said it. */ diff --git a/packages/spec/src/automation/flow.test.ts b/packages/spec/src/automation/flow.test.ts index 379e7d7df8d..e53fc907aad 100644 --- a/packages/spec/src/automation/flow.test.ts +++ b/packages/spec/src/automation/flow.test.ts @@ -788,8 +788,11 @@ describe('FlowSchema', () => { id: 'process_response', type: 'script', label: 'Process Response', + // A registered function, the one thing a `script` node runs — the + // inline `script` key was retired, and the flow parse now refuses + // a `script` node with no `function` (the key its contract requires). config: { - script: 'return JSON.parse(response.body);', + function: 'parse_response', }, }, { id: 'end', type: 'end', label: 'End' }, @@ -1106,7 +1109,7 @@ describe('BPMN — Parallel Gateway & Join Gateway', () => { { id: 'finance_review', type: 'connector_action', label: 'Finance Review' }, { id: 'legal_review', type: 'connector_action', label: 'Legal Review' }, { id: 'join', type: 'join_gateway', label: 'Join — All Approved' }, - { id: 'final_approve', type: 'update_record', label: 'Final Approve' }, + { id: 'final_approve', type: 'update_record', label: 'Final Approve', config: { objectName: 'contract', filter: { id: '{record.id}' }, fields: { status: 'approved' } } }, { id: 'end', type: 'end', label: 'End' }, ], edges: [ @@ -1193,9 +1196,9 @@ describe('BPMN — Default Sequence Flow (isDefault)', () => { nodes: [ { id: 'start', type: 'start', label: 'Start' }, { id: 'check_priority', type: 'decision', label: 'Check Priority' }, - { id: 'high_path', type: 'update_record', label: 'High Priority Handler' }, - { id: 'medium_path', type: 'update_record', label: 'Medium Priority Handler' }, - { id: 'default_path', type: 'update_record', label: 'Default Handler' }, + { id: 'high_path', type: 'update_record', label: 'High Priority Handler', config: { objectName: 'ticket', filter: { id: '{record.id}' } } }, + { id: 'medium_path', type: 'update_record', label: 'Medium Priority Handler', config: { objectName: 'ticket', filter: { id: '{record.id}' } } }, + { id: 'default_path', type: 'update_record', label: 'Default Handler', config: { objectName: 'ticket', filter: { id: '{record.id}' } } }, { id: 'end', type: 'end', label: 'End' }, ], edges: [ @@ -1610,7 +1613,7 @@ describe('BPMN — Boundary Event', () => { type: 'autolaunched', nodes: [ { id: 'start', type: 'start', label: 'Start' }, - { id: 'api_call', type: 'http', label: 'Call External API', timeoutMs: 5000 }, + { id: 'api_call', type: 'http', label: 'Call External API', timeoutMs: 5000, config: { url: 'https://api.example.com/v1/sync' } }, { id: 'api_error_boundary', type: 'boundary_event', @@ -1622,7 +1625,7 @@ describe('BPMN — Boundary Event', () => { errorCode: 'TIMEOUT', }, }, - { id: 'handle_error', type: 'update_record', label: 'Log Error' }, + { id: 'handle_error', type: 'update_record', label: 'Log Error', config: { objectName: 'sync_log', filter: { id: '{record.id}' } } }, { id: 'end_success', type: 'end', label: 'End Success' }, { id: 'end_error', type: 'end', label: 'End Error' }, ], diff --git a/packages/spec/src/automation/flow.zod.ts b/packages/spec/src/automation/flow.zod.ts index 982c6cbda4a..d2b34efb0ec 100644 --- a/packages/spec/src/automation/flow.zod.ts +++ b/packages/spec/src/automation/flow.zod.ts @@ -23,7 +23,8 @@ import { retiredKey } from '../shared/retired-key'; import { retryPolicyShape } from '../shared/retry-policy.zod'; import { strictObject } from '../shared/strict-object'; import { collectFlowGraphs, parseFlowNodeRegions } from './control-flow.zod'; -import { flowNodeConfigRefusals, predicateSlotRefusal, resolveFlowNodeExpressions } from './flow-node-expression-paths'; +import { predicateSlotRefusal, resolveFlowNodeExpressions } from './flow-node-expression-paths'; +import { flowNodeConfigRefusals } from './flow-node-config-refusals'; import { EndConfigSchema } from './builtin-node-config.zod'; import { APPROVAL_NODE_TYPE, APPROVAL_REVISE_NODE_TYPE } from './approval.zod'; export const FlowNodeAction = z.enum([ diff --git a/packages/spec/src/automation/index.ts b/packages/spec/src/automation/index.ts index 5b579173375..034230b13d3 100644 --- a/packages/spec/src/automation/index.ts +++ b/packages/spec/src/automation/index.ts @@ -69,5 +69,6 @@ export * from './schedule-organization.zod'; export * from './state-machine.zod'; export * from './node-executor.zod'; export * from './flow-node-expression-paths'; +export * from './flow-node-config-refusals'; export * from './bpmn-interop.zod'; export * from './bpmn-mapping'; diff --git a/packages/spec/src/automation/region-normalization.test.ts b/packages/spec/src/automation/region-normalization.test.ts index 8b1a940d844..ecd1b06609b 100644 --- a/packages/spec/src/automation/region-normalization.test.ts +++ b/packages/spec/src/automation/region-normalization.test.ts @@ -32,7 +32,10 @@ const CONDITION = 'row.shouldRun == true'; const ENVELOPE = { dialect: 'cel', source: CONDITION }; const gate = { id: 'gate', type: 'decision', label: 'Gate' }; -const write = { id: 'write', type: 'create_record', label: 'Write' }; +// `objectName` is the key the create_record executor contract requires; a +// region fixture carries it so the flow parse judges only what these tests +// pin (#20316 refuses a node config that leaves it out). +const write = { id: 'write', type: 'create_record', label: 'Write', config: { objectName: 'task' } }; /** * A well-formed region whose single edge carries a BARE STRING condition. * @@ -239,29 +242,37 @@ describe('#4347 — collectFlowGraphs', () => { })).map(g => g.scope)).toEqual(['', "try_catch 'tc' try", "try_catch 'tc' catch"]); }); + // The nested `try_catch` carries its `try` (#20316: the try_catch executor + // contract requires it, and the flow parse now refuses one left out), so the + // chain runs through BOTH of its regions. it('chains the scope of a nested region so a finding says where it is', () => { const flow = flowWith(loopWith({ nodes: [{ id: 'tc', type: TRY_CATCH_NODE_TYPE, label: 'Guard', - config: { catch: gatedRegion() }, + config: { try: gatedRegion('_t'), catch: gatedRegion() }, }], edges: [], })); - expect(collectFlowGraphs(flow).map(g => g.scope)) - .toEqual(['', "loop 'loop' body", "loop 'loop' body → try_catch 'tc' catch"]); + expect(collectFlowGraphs(flow).map(g => g.scope)).toEqual([ + '', + "loop 'loop' body", + "loop 'loop' body → try_catch 'tc' try", + "loop 'loop' body → try_catch 'tc' catch", + ]); }); it('carries each graph\'s key path beside its scope, so a finding can be anchored where the author wrote it (#16134)', () => { const flow = flowWith(loopWith({ nodes: [{ id: 'tc', type: TRY_CATCH_NODE_TYPE, label: 'Guard', - config: { catch: gatedRegion() }, + config: { try: gatedRegion('_t'), catch: gatedRegion() }, }], edges: [], })); expect(collectFlowGraphs(flow).map(g => g.path)).toEqual([ [], ['nodes', 1, 'config', 'body'], + ['nodes', 1, 'config', 'body', 'nodes', 0, 'config', 'try'], ['nodes', 1, 'config', 'body', 'nodes', 0, 'config', 'catch'], ]); expect(collectFlowGraphs(flowWith({ diff --git a/packages/spec/src/conversions/conversions.test.ts b/packages/spec/src/conversions/conversions.test.ts index 2a284e4731d..9795df753c1 100644 --- a/packages/spec/src/conversions/conversions.test.ts +++ b/packages/spec/src/conversions/conversions.test.ts @@ -590,9 +590,17 @@ describe('conversion layer (ADR-0087 D2)', () => { `${key} must be rejected`, ).toThrow(/was removed in @objectstack\/spec 17/); } - // The flow-level parse is deliberately blind here — pinned so the note - // above stays true if `FlowNodeSchema.config` is ever tightened. - expect(() => FlowSchema.parse((scriptFlow({ actionType: 'email' }).flows as any[])[0])).not.toThrow(); + // The flow-level parse is deliberately blind to the TOMBSTONES — pinned so + // the note above stays true if `FlowNodeSchema.config` is ever tightened. + // What it does see since #20316 is the one key the script contract + // requires, left out: a stripped node naming no callable is refused at + // the build doors now, not only at execute — and by that, never by a + // tombstone. + expect(() => FlowSchema.parse((scriptFlow({ function: 'score_lead', actionType: 'email' }).flows as any[])[0])).not.toThrow(); + const stripped = FlowSchema.safeParse((scriptFlow({ actionType: 'email' }).flows as any[])[0]); + expect(stripped.success).toBe(false); + expect(stripped.error!.issues.map((i) => i.path.join('.'))).toEqual(['nodes.1.config.function']); + expect(stripped.error!.issues[0].message).not.toMatch(/was removed in @objectstack\/spec 17/); }); }); From a51d8487d67009c8219004d170ca8a54b00cd205 Mon Sep 17 00:00:00 2001 From: Claude Date: Mon, 28 Sep 2026 05:41:13 +0000 Subject: [PATCH 4/8] wip: changeset, regenerated api-surface / export-origins / skill reference index Claude-Session: https://claude.ai/code/session_01QcAS3qiYYZNezaxZxaUdMV Co-Authored-By: Claude --- ...-flow-node-config-required-keys-refused.md | 82 +++++++++++++++++++ packages/spec/api-surface/automation.json | 6 ++ packages/spec/export-origins/automation.json | 6 ++ .../references/_index.md | 1 + 4 files changed, 95 insertions(+) create mode 100644 .changeset/20316-flow-node-config-required-keys-refused.md diff --git a/.changeset/20316-flow-node-config-required-keys-refused.md b/.changeset/20316-flow-node-config-required-keys-refused.md new file mode 100644 index 00000000000..d3e8c52ef6d --- /dev/null +++ b/.changeset/20316-flow-node-config-required-keys-refused.md @@ -0,0 +1,82 @@ +--- +'@objectstack/spec': minor +'@objectstack/lint': minor +--- + +fix(spec)!: a flow node config its executor cannot run — a key its contract requires, left out, or a decision branch list it cannot read — is refused at authoring (#20316) + +Clause-②: no (narrowing) + + + +**BREAKING** — an accept-set narrowing on authored flow-node `config`, shipped as +`minor` under the launch-window convention (`check-changeset-no-major` refuses +`major` until GA; breaking-ness is carried by this banner and the ADR-0087 +disposition above, not by the level). + +**What changed.** A flow node's `config` is an open record, so what its executor +requires was checked by no build door. `FlowSchema.parse`, `AutomationEngine.registerFlow` +and `objectstack validate` all admitted a node that left out a key its executor +contract requires — and the executor's own contract parse then refused the node on +every run that reached it. A `decision` branch with no `label` was worse: it never +failed, the matched branch reported no label, and traversal took EVERY out-edge, so +the flow ran green down the wrong paths. All three doors now refuse these shapes +through one judge, `flowNodeConfigRefusals` (new in `@objectstack/spec/automation`): + +- **A key a builtin's executor contract requires, left out.** Each builtin node's + config is parsed against the very contract its executor parses against + (`getBuiltinNodeConfigContracts()`, new, reconciled against the executors' own parse + calls), and only the keys left out are kept — a present value of the wrong type, and + an undeclared key, are judged where they were before. The keys: `objectName` on + `get_record` / `create_record` / `update_record` / `delete_record`; `recipients` on + `notify` (and `title` when there is no `template`); `url` on `http`; `function` on + `script`; `flowName` on `subflow`; `collection` and `flowName` on `map`; + `collection` on a `loop` that has a `body`; `branches` on `parallel`; `try` on + `try_catch`; and on `screen`, each field's `name`, each option's `value` and + `label`, and a `lookup` field's `reference`. A key a rule of the contract requires + (the `notify` title, the `lookup` reference) is refused in the contract's own words. +- **A `decision` branch list its executor cannot read.** `conditions` present and not + `null` must be an array; every branch must be an object; every branch's `label` must + be a non-blank string (absent, `null`, blank or non-text all name no out-edge). + +Each refusal is a `custom` issue anchored at the key (`nodes.1.config.objectName`, +`nodes.1.config.fields.0.name`, `nodes.1.config.conditions.0.label`, or the region +path `nodes.1.config.body.nodes.0.config…`), met at `registerFlow` and +`objectstack validate` through that same parse, and reported by +`validateStackExpressions` for a stack handed to it directly. The refusal codes join +`FLOW_SLOT_REFUSAL_CODES`: `node-config-key-missing`, `node-config-key-required-by-rule`, +`decision-conditions-not-array`, `decision-branch-not-object`, +`decision-branch-label-missing`. + +The Studio flow designer writes refused shapes when a node is added and saved before +it is configured, when a decision branch row's label cell is left empty, and when a +screen field row's name cell is left empty. Where such a node already sits, the whole +flow is refused: registered from the metadata registry or `sys_metadata` at boot, it +is skipped with a `failed to register flow` warn naming it while the flows beside it +register; a `defineStack({ flows })` source throws `StackSchemaInvalidError` for the +whole stack; an artifact file is refused whole at load. + +## FROM → TO + +| you wrote | write instead | +|:--|:--| +| `{ type: 'get_record', config: { outputVariable: 'rows' } }` | the object it reads — `config: { objectName: 'account', outputVariable: 'rows' }` (the same for `create_record` / `update_record` / `delete_record`) | +| `{ type: 'loop', config: { body: { … } } }` | the array it iterates — `config: { collection: '{rows}', body: { … } }` | +| `{ type: 'map', config: { flowName: 'per_row' } }` | `config: { collection: '{rows}', flowName: 'per_row' }` | +| `{ type: 'http', config: { method: 'GET' } }` | `config: { url: 'https://api.example.com/v1/items', method: 'GET' }` | +| `{ type: 'script' }` | the registered function it calls — `config: { function: 'recalc_totals' }` | +| `{ type: 'notify', config: { recipients: ['{record.owner}'] } }` | a content source — `title: 'Deal won'`, or a `template` | +| `conditions: [{ expression: 'record.amount > 1000' }]` on a `decision` | the out-edge it routes to — `[{ label: 'large', expression: 'record.amount > 1000' }]`, beside an out-edge labelled `large` | +| `conditions: ['record.amount > 1000']` | `[{ label: 'large', expression: 'record.amount > 1000' }]` | + +**One-line fix:** write the key the node was meant to carry. To branch on the +out-edges instead of on `conditions`, delete `conditions` and put each predicate on its +edge's `condition`. + +**Unchanged.** A node carrying every key its contract requires parses, registers and +validates as before; a legacy flat-graph `loop` (no `body`) still needs no +`collection`; a `decision` with no `conditions`, `conditions: null` or an empty list +still routes by its out-edges; `assignment`, `wait`, `connector_action` and plugin node +types are not judged by this rule; and a key spelled by a D2 alias (`object`, `flow`, +`functionName`, …) is still canonicalized before `registerFlow` and `objectstack +validate` judge it. diff --git a/packages/spec/api-surface/automation.json b/packages/spec/api-surface/automation.json index ba1ca8ba349..265d3369946 100644 --- a/packages/spec/api-surface/automation.json +++ b/packages/spec/api-surface/automation.json @@ -68,6 +68,7 @@ "BpmnUnmappedStrategySchema (const)", "BpmnVersion (type)", "BpmnVersionSchema (const)", + "BuiltinNodeConfigContract (interface)", "Checkpoint (type)", "CheckpointParsed (type)", "CheckpointSchema (const)", @@ -138,6 +139,8 @@ "FlowNode (type)", "FlowNodeAction (const)", "FlowNodeAction (type)", + "FlowNodeConfigRefusal (type)", + "FlowNodeConfigRefusalCode (type)", "FlowNodeExpressionPath (interface)", "FlowNodeExpressionRole (type)", "FlowNodeParsed (type)", @@ -186,6 +189,7 @@ "MapConfigSchema (const)", "MappableFlow (interface)", "NON_AUTHORABLE_APPROVER_TYPES (const)", + "NodeConfigValueKind (type)", "NodeExecutorDescriptor (type)", "NodeExecutorDescriptorParsed (type)", "NodeExecutorDescriptorSchema (const)", @@ -281,7 +285,9 @@ "exportConstructsToBpmn (function)", "findRegionEntry (function)", "flowForm (const)", + "flowNodeConfigRefusals (function)", "getApprovalNodeConfigJsonSchema (function)", + "getBuiltinNodeConfigContracts (function)", "getSchemalessNodeConfigJsonSchemas (function)", "importBpmnToConstructs (function)", "isExpressionEnvelopeShaped (function)", diff --git a/packages/spec/export-origins/automation.json b/packages/spec/export-origins/automation.json index 78087ab9fae..183af79e6bd 100644 --- a/packages/spec/export-origins/automation.json +++ b/packages/spec/export-origins/automation.json @@ -66,6 +66,7 @@ "BpmnUnmappedStrategySchema": "src/automation/bpmn-interop.zod.ts#BpmnUnmappedStrategySchema (const)", "BpmnVersion": "src/automation/bpmn-interop.zod.ts#BpmnVersion (type)", "BpmnVersionSchema": "src/automation/bpmn-interop.zod.ts#BpmnVersionSchema (const)", + "BuiltinNodeConfigContract": "src/automation/flow-node-config-refusals.ts#BuiltinNodeConfigContract (interface)", "Checkpoint": "src/automation/execution.zod.ts#Checkpoint (type)", "CheckpointParsed": "src/automation/execution.zod.ts#CheckpointParsed (type)", "CheckpointSchema": "src/automation/execution.zod.ts#CheckpointSchema (const)", @@ -133,6 +134,8 @@ "FlowGraph": "src/automation/control-flow.zod.ts#FlowGraph (interface)", "FlowNode": "src/automation/flow.zod.ts#FlowNode (type)", "FlowNodeAction": "src/automation/flow.zod.ts#FlowNodeAction (type)", + "FlowNodeConfigRefusal": "src/automation/flow-node-expression-paths.ts#FlowNodeConfigRefusal (type)", + "FlowNodeConfigRefusalCode": "src/automation/flow-node-expression-paths.ts#FlowNodeConfigRefusalCode (type)", "FlowNodeExpressionPath": "src/automation/flow-node-expression-paths.ts#FlowNodeExpressionPath (interface)", "FlowNodeExpressionRole": "src/automation/flow-node-expression-paths.ts#FlowNodeExpressionRole (type)", "FlowNodeParsed": "src/automation/flow.zod.ts#FlowNodeParsed (type)", @@ -181,6 +184,7 @@ "MapConfigSchema": "src/automation/builtin-node-config.zod.ts#MapConfigSchema (const)", "MappableFlow": "src/automation/bpmn-mapping.ts#MappableFlow (interface)", "NON_AUTHORABLE_APPROVER_TYPES": "src/automation/approval.zod.ts#NON_AUTHORABLE_APPROVER_TYPES (const)", + "NodeConfigValueKind": "src/automation/flow-node-expression-paths.ts#NodeConfigValueKind (type)", "NodeExecutorDescriptor": "src/automation/node-executor.zod.ts#NodeExecutorDescriptor (type)", "NodeExecutorDescriptorParsed": "src/automation/node-executor.zod.ts#NodeExecutorDescriptorParsed (type)", "NodeExecutorDescriptorSchema": "src/automation/node-executor.zod.ts#NodeExecutorDescriptorSchema (const)", @@ -275,7 +279,9 @@ "exportConstructsToBpmn": "src/automation/bpmn-mapping.ts#exportConstructsToBpmn (function)", "findRegionEntry": "src/automation/control-flow.zod.ts#findRegionEntry (function)", "flowForm": "src/automation/flow.form.ts#flowForm (const)", + "flowNodeConfigRefusals": "src/automation/flow-node-config-refusals.ts#flowNodeConfigRefusals (function)", "getApprovalNodeConfigJsonSchema": "src/automation/approval.zod.ts#getApprovalNodeConfigJsonSchema (function)", + "getBuiltinNodeConfigContracts": "src/automation/flow-node-config-refusals.ts#getBuiltinNodeConfigContracts (function)", "getSchemalessNodeConfigJsonSchemas": "src/automation/schemaless-node-config.zod.ts#getSchemalessNodeConfigJsonSchemas (function)", "importBpmnToConstructs": "src/automation/bpmn-mapping.ts#importBpmnToConstructs (function)", "isExpressionEnvelopeShaped": "src/automation/flow-node-expression-paths.ts#isExpressionEnvelopeShaped (function)", diff --git a/skills/objectstack-automation/references/_index.md b/skills/objectstack-automation/references/_index.md index bf02f4f4688..c0d5a76f2fd 100644 --- a/skills/objectstack-automation/references/_index.md +++ b/skills/objectstack-automation/references/_index.md @@ -22,6 +22,7 @@ from `node_modules` — there is no local copy in the skill bundle. ## Transitive dependencies - `node_modules/@objectstack/spec/src/automation/control-flow.zod.ts` — Structured control-flow constructs (ADR-0031) — the **native + AI-authored** +- `node_modules/@objectstack/spec/src/automation/schemaless-node-config.zod.ts` — Config contracts for the **descriptor-schemaless** builtins whose designer - `node_modules/@objectstack/spec/src/kernel/metadata-protection.zod.ts` — Metadata Protection Model — Phase 1 (ADR-0010) - `node_modules/@objectstack/spec/src/shared/expression.zod.ts` — Expression Protocol - `node_modules/@objectstack/spec/src/shared/identifiers.zod.ts` — Exports: SystemIdentifierSchema, SnakeCaseIdentifierSchema, MetadataItemNameSchema From 40575ae941b134ea1fc72c7509cdeab795258d9d Mon Sep 17 00:00:00 2001 From: Claude Date: Mon, 28 Sep 2026 05:58:02 +0000 Subject: [PATCH 5/8] wip: the judge skips a key missing inside an authored value slot; re-judge the executor-refusal pins as register-whole-then-strip Claude-Session: https://claude.ai/code/session_01QcAS3qiYYZNezaxZxaUdMV Co-Authored-By: Claude --- .../src/builtin/config-parse.test.ts | 82 +++++++++++++------ .../src/builtin/http-nodes.test.ts | 7 +- .../src/builtin/notify-node.test.ts | 14 +++- .../src/builtin/screen-nodes.test.ts | 13 +-- .../src/builtin/subflow-node.test.ts | 7 +- .../service-automation/src/engine.test.ts | 4 +- .../src/fault-edge-guard-containment.test.ts | 22 ++--- .../src/flow-activation-ledger.test.ts | 3 +- .../src/flow-retry-attempt-count.test.ts | 2 +- .../src/guard-refusal-inventory.test.ts | 41 +++++++--- .../src/input-schema-retry-parity.test.ts | 4 +- .../automation/flow-node-config-refusals.ts | 23 +++++- .../flow-node-config-required.test.ts | 8 ++ 13 files changed, 164 insertions(+), 66 deletions(-) diff --git a/packages/services/service-automation/src/builtin/config-parse.test.ts b/packages/services/service-automation/src/builtin/config-parse.test.ts index bacac967f0c..59e4d80d379 100644 --- a/packages/services/service-automation/src/builtin/config-parse.test.ts +++ b/packages/services/service-automation/src/builtin/config-parse.test.ts @@ -79,6 +79,36 @@ function flowWith( }; } +/** + * #20316 — a key the executor contract requires, LEFT OUT, is refused at the + * build doors now: `registerFlow` parses first, so a flow missing it no longer + * registers ({@link doorRefusal} pins that half). The execute-time parse this + * file is about is still the executor's own contract, met by a config that + * reaches it past the doors — so {@link runStripped} registers the node WHOLE + * and removes the keys from the stored flow before the run. + */ +function doorRefusal(type: string, config: Record): string { + try { + engineWith().registerFlow('f', flowWith(type, config)); + } catch (e) { + return String((e as Error).message); + } + return ''; +} + +async function runStripped( + engine: AutomationEngine, + type: string, + whole: Record, + strip: string[], + extra?: { nodes?: any[]; edges?: any[]; variables?: any[] }, +) { + const stored = engine.registerFlow('f', flowWith(type, whole, extra)); + const config = stored.nodes.find((n) => n.id === 'n1')!.config as Record; + for (const key of strip) delete config[key]; + return engine.execute('f'); +} + describe('execute-time config parse (#4277)', () => { it('refuses a wrong-typed declared key, naming the exact path', async () => { const engine = engineWith(); @@ -112,10 +142,9 @@ describe('execute-time config parse (#4277)', () => { }); it('refuses a missing required key (notify without title)', async () => { + expect(doorRefusal('notify', { recipients: 'u1' })).toContain('A notify node needs one content source'); const engine = engineWith(); - engine.registerFlow('f', flowWith('notify', { recipients: 'u1' })); - - const result = await engine.execute('f'); + const result = await runStripped(engine, 'notify', { recipients: 'u1', title: 'Hi' }, ['title']); expect(result.success).toBe(false); expect(result.error).toContain('notify'); expect(result.error).toContain('config.title'); @@ -177,12 +206,10 @@ describe('execute-time config parse (#4277)', () => { }); it('a structured loop (body present) IS parsed — missing collection refuses', async () => { + const body = { nodes: [{ id: 'b1', type: 'assignment', label: 'B', config: { x: 1 } }], edges: [] }; + expect(doorRefusal('loop', { body })).toContain("config leaves out `collection`"); const engine = engineWith(); - engine.registerFlow('f', flowWith('loop', { - body: { nodes: [{ id: 'b1', type: 'assignment', label: 'B', config: { x: 1 } }], edges: [] }, - })); - - const result = await engine.execute('f'); + const result = await runStripped(engine, 'loop', { collection: [1], body }, ['collection']); expect(result.success).toBe(false); expect(result.error).toContain('loop'); expect(result.error).toContain('config.collection'); @@ -201,10 +228,9 @@ describe('execute-time config parse (#4277)', () => { }); it('map refuses a missing collection, naming the path', async () => { + expect(doorRefusal('map', { flowName: 'child' })).toContain("config leaves out `collection`"); const engine = engineWith(); - engine.registerFlow('f', flowWith('map', { flowName: 'child' })); - - const result = await engine.execute('f'); + const result = await runStripped(engine, 'map', { flowName: 'child', collection: [1] }, ['collection']); expect(result.success).toBe(false); expect(result.error).toContain('map'); expect(result.error).toContain('config.collection'); @@ -218,10 +244,9 @@ describe('execute-time config parse (#4277)', () => { // always flat — it just carried a hand-written guard instead of the contract. it('script refuses a node that names no callable', async () => { + expect(doorRefusal('script', {})).toContain("config leaves out `function`"); const engine = engineWith(); - engine.registerFlow('f', flowWith('script', {})); - - const result = await engine.execute('f'); + const result = await runStripped(engine, 'script', { function: 'recalc' }, ['function']); expect(result.success).toBe(false); expect(result.error).toContain('does not satisfy the script contract'); expect(result.error).toContain('config.function'); @@ -231,28 +256,32 @@ describe('execute-time config parse (#4277)', () => { const engine = engineWith(); // `registerFlow` strips the retired keys on rehydration (#3903), so what // reaches the parse is a node with nothing to run. Before #4343 this was a - // green step that delivered no mail. - engine.registerFlow('f', flowWith('script', { + // green step that delivered no mail. Since #20316 the flow parse behind + // that conversion refuses the stripped node itself — it names no + // `function` — so the flow no longer registers; the run below meets the + // same stripped shape past the doors. + expect(doorRefusal('script', { actionType: 'email', template: 'task_done', recipients: ['{record.owner}'], - })); - - const result = await engine.execute('f'); + })).toContain("config leaves out `function`"); + const result = await runStripped(engine, 'script', { + function: 'send_mail', actionType: 'email', template: 'task_done', recipients: ['{record.owner}'], + }, ['function']); expect(result.success).toBe(false); expect(result.error).toContain('does not satisfy the script contract'); }); it('a script parse refusal is a guard — a fault edge does NOT route it', async () => { const engine = engineWith(); - engine.registerFlow('f', flowWith( + const result = await runStripped( + engine, 'script', - { actionType: 'slack', template: 't' }, + { function: 'post_to_slack', actionType: 'slack', template: 't' }, + ['function'], { nodes: [{ id: 'recover', type: 'assignment', label: 'R', config: { recovered: true } }], edges: [{ id: 'e3', source: 'n1', target: 'recover', type: 'fault' }], }, - )); - - const result = await engine.execute('f'); + ); expect(result.success).toBe(false); expect(result.error).toContain('does not satisfy the script contract'); }); @@ -275,10 +304,9 @@ describe('execute-time config parse (#4277)', () => { }); it('subflow refuses a missing flowName through the contract, not a hand-written check', async () => { + expect(doorRefusal('subflow', {})).toContain("config leaves out `flowName`"); const engine = engineWith(); - engine.registerFlow('f', flowWith('subflow', {})); - - const result = await engine.execute('f'); + const result = await runStripped(engine, 'subflow', { flowName: 'child' }, ['flowName']); expect(result.success).toBe(false); expect(result.error).toContain('does not satisfy the subflow contract'); expect(result.error).toContain('config.flowName'); diff --git a/packages/services/service-automation/src/builtin/http-nodes.test.ts b/packages/services/service-automation/src/builtin/http-nodes.test.ts index 547d3d4f0e9..7e2b425b53e 100644 --- a/packages/services/service-automation/src/builtin/http-nodes.test.ts +++ b/packages/services/service-automation/src/builtin/http-nodes.test.ts @@ -213,7 +213,12 @@ describe('http (canonical node)', () => { it('fails the step when url is missing', async () => { const engine = new AutomationEngine(createTestLogger()); registerHttpNodes(engine, createCtx()); - engine.registerFlow('http_flow', httpFlow('http', { method: 'GET' })); + // #20316 — `url` is the key the http contract requires, so a node + // without it is refused at registration now… + expect(() => engine.registerFlow('http_flow', httpFlow('http', { method: 'GET' }))).toThrow(/leaves out `url`/); + // …and the executor still refuses one that reaches it past the doors. + const stored = engine.registerFlow('http_flow', httpFlow('http', { method: 'GET', url: 'https://example.invalid/x' })); + delete (stored.nodes.find((n) => n.id === 'http')!.config as Record).url; const result = await engine.execute('http_flow'); expect(result.success).toBe(false); expect(result.error).toContain('url'); diff --git a/packages/services/service-automation/src/builtin/notify-node.test.ts b/packages/services/service-automation/src/builtin/notify-node.test.ts index 84ee5af6332..7f1b32d32e7 100644 --- a/packages/services/service-automation/src/builtin/notify-node.test.ts +++ b/packages/services/service-automation/src/builtin/notify-node.test.ts @@ -228,7 +228,13 @@ describe('notify (baseline node)', () => { }); it('fails the step when title is missing', async () => { - engine.registerFlow('notify_flow', notifyFlow({ recipients: ['user_1'] })); + // #20316 — with no `template`, `title` is a key the notify contract + // requires, so the node is refused at registration now, in the + // contract's own words… + expect(() => engine.registerFlow('notify_flow', notifyFlow({ recipients: ['user_1'] }))).toThrow(/A notify node needs one content source/); + // …and the executor still refuses one that reaches it past the doors. + const stored = engine.registerFlow('notify_flow', notifyFlow({ recipients: ['user_1'], title: 'Hi' })); + delete (stored.nodes.find((n) => n.id === 'notify')!.config as Record).title; const result = await engine.execute('notify_flow'); expect(result.success).toBe(false); expect(result.error).toContain('title'); @@ -278,7 +284,11 @@ describe('notify (baseline node)', () => { }); it('fails the step when no recipient is given', async () => { - engine.registerFlow('notify_flow', notifyFlow({ title: 'Hi' })); + // #20316 — refused at registration now (`recipients` is required)… + expect(() => engine.registerFlow('notify_flow', notifyFlow({ title: 'Hi' }))).toThrow(/leaves out `recipients`/); + // …and the executor still refuses one that reaches it past the doors. + const stored = engine.registerFlow('notify_flow', notifyFlow({ title: 'Hi', recipients: ['user_1'] })); + delete (stored.nodes.find((n) => n.id === 'notify')!.config as Record).recipients; const result = await engine.execute('notify_flow'); expect(result.success).toBe(false); expect(result.error).toContain('recipient'); diff --git a/packages/services/service-automation/src/builtin/screen-nodes.test.ts b/packages/services/service-automation/src/builtin/screen-nodes.test.ts index 5eb1789da4f..094907191bb 100644 --- a/packages/services/service-automation/src/builtin/screen-nodes.test.ts +++ b/packages/services/service-automation/src/builtin/screen-nodes.test.ts @@ -138,6 +138,12 @@ it('canonicalizes a stored `functionName` key to `function` at load (#1870 DX, # * names no callable — so the node refuses, loudly, where it used to log a line * and report success. That flip is the whole point of the retirement, and it is * what these cases pin. + * + * Since #20316 the refusal lands one door earlier: `function` is the key the + * script contract requires, and the flow parse behind the conversion refuses a + * script node that leaves it out — so the stored flow no longer REGISTERS (at + * boot: skipped with a `failed to register flow` warn naming it), rather than + * registering and refusing at run time. Same verdict, named the same way. */ describe('script retired branches, as a stored flow meets them (#4343)', () => { let engine: AutomationEngine; @@ -155,11 +161,8 @@ describe('script retired branches, as a stored flow meets them (#4343)', () => { ['an inline body', { script: 'return { ok: true };' }], ['the bare marker', { actionType: 'invoke_function' }], ] as const)('%s no longer succeeds silently — it refuses, naming the callable it lacks', async (_name, config) => { - engine.registerFlow('script_flow', scriptFlow({ ...config })); - const result = await engine.execute('script_flow', {} as any); - expect(result.success).toBe(false); - expect(result.error).toContain('does not satisfy the script contract'); - expect(result.error).toContain('config.function'); + expect(() => engine.registerFlow('script_flow', scriptFlow({ ...config }))).toThrow(/config leaves out `function`/); + expect(await engine.getFlow('script_flow')).toBeNull(); }); it('converts a shorthand `actionType` into the function it always named, and runs it', async () => { diff --git a/packages/services/service-automation/src/builtin/subflow-node.test.ts b/packages/services/service-automation/src/builtin/subflow-node.test.ts index 42725c8b326..f0b871cee39 100644 --- a/packages/services/service-automation/src/builtin/subflow-node.test.ts +++ b/packages/services/service-automation/src/builtin/subflow-node.test.ts @@ -141,7 +141,12 @@ describe('subflow node executor', () => { }); it('fails with a clear error when flowName is missing', async () => { - engine.registerFlow('parent_flow', parentFlow({ input: {} })); + // #20316 — `flowName` is the key the subflow contract requires, so the + // node is refused at registration now… + expect(() => engine.registerFlow('parent_flow', parentFlow({ input: {} }))).toThrow(/leaves out `flowName`/); + // …and the executor still refuses one that reaches it past the doors. + const stored = engine.registerFlow('parent_flow', parentFlow({ input: {}, flowName: 'child_flow' })); + delete (stored.nodes.find((n) => n.type === 'subflow')!.config as Record).flowName; const result = await engine.execute('parent_flow'); expect(result.success).toBe(false); // #4343 — the hand-written guard became the contract parse; same guard diff --git a/packages/services/service-automation/src/engine.test.ts b/packages/services/service-automation/src/engine.test.ts index a633b1e73b0..613911d9034 100644 --- a/packages/services/service-automation/src/engine.test.ts +++ b/packages/services/service-automation/src/engine.test.ts @@ -1433,7 +1433,7 @@ describe('AutomationEngine - Execution History', () => { name: 'failing_flow', nodes: [ { id: 'start', type: 'start' as const, label: 'Start' }, - { id: 'bad', type: 'script' as const, label: 'Bad' }, + { id: 'bad', type: 'script' as const, label: 'Bad', config: { function: 'noop' } }, { id: 'end', type: 'end' as const, label: 'End' }, ], edges: [ @@ -3167,7 +3167,7 @@ describe('#9378 — execute() classifies terminal exits for the trigger transpor name, label: name, type: 'autolaunched' as const, nodes: [ { id: 'start', type: 'start' as const, label: 'Start' }, - { id: 'bad', type: 'script' as const, label: 'Bad' }, + { id: 'bad', type: 'script' as const, label: 'Bad', config: { function: 'noop' } }, { id: 'end', type: 'end' as const, label: 'End' }, ], edges: [ diff --git a/packages/services/service-automation/src/fault-edge-guard-containment.test.ts b/packages/services/service-automation/src/fault-edge-guard-containment.test.ts index 7dc29650929..2c8aabab02f 100644 --- a/packages/services/service-automation/src/fault-edge-guard-containment.test.ts +++ b/packages/services/service-automation/src/fault-edge-guard-containment.test.ts @@ -81,7 +81,7 @@ describe('#3863 — a fault edge must not swallow a guard refusal', () => { label: 'Delete', config: { objectName: 'deal', filter: { owner: '{record.ownr}' } }, }, - { id: 'handler', type: 'script' as any, label: 'Handler' }, + { id: 'handler', type: 'script' as any, label: 'Handler', config: { function: 'noop' } }, { id: 'end', type: 'end' as const, label: 'End' }, ], edges: [ @@ -136,8 +136,8 @@ describe('#3863 — a fault edge must not swallow a guard refusal', () => { type: 'autolaunched', nodes: [ { id: 'start', type: 'start', label: 'Start' }, - { id: 'risky', type: 'script' as any, label: 'Risky' }, - { id: 'handler', type: 'script' as any, label: 'Handler' }, + { id: 'risky', type: 'script' as any, label: 'Risky', config: { function: 'noop' } }, + { id: 'handler', type: 'script' as any, label: 'Handler', config: { function: 'noop' } }, { id: 'end', type: 'end', label: 'End' }, ], edges: [ @@ -178,7 +178,7 @@ describe('#3863 — a fault edge must not swallow a guard refusal', () => { label: 'Delete', config: { objectName: 'deal', filter: { status: 'closed' } }, }, - { id: 'handler', type: 'script' as any, label: 'Handler' }, + { id: 'handler', type: 'script' as any, label: 'Handler', config: { function: 'noop' } }, { id: 'end', type: 'end', label: 'End' }, ], edges: [ @@ -218,8 +218,8 @@ describe('#3863 — a fault edge must not swallow a guard refusal', () => { type: 'autolaunched', nodes: [ { id: 'start', type: 'start', label: 'Start' }, - { id: 'risky', type: 'script' as any, label: 'Risky' }, - { id: 'handler', type: 'script' as any, label: 'Handler' }, + { id: 'risky', type: 'script' as any, label: 'Risky', config: { function: 'noop' } }, + { id: 'handler', type: 'script' as any, label: 'Handler', config: { function: 'noop' } }, { id: 'end', type: 'end', label: 'End' }, ], edges: [ @@ -252,8 +252,8 @@ describe('#3863 — a fault edge must not swallow a guard refusal', () => { type: 'autolaunched', nodes: [ { id: 'start', type: 'start', label: 'Start' }, - { id: 'risky', type: 'script' as any, label: 'Risky' }, - { id: 'handler', type: 'script' as any, label: 'Handler' }, + { id: 'risky', type: 'script' as any, label: 'Risky', config: { function: 'noop' } }, + { id: 'handler', type: 'script' as any, label: 'Handler', config: { function: 'noop' } }, { id: 'end', type: 'end', label: 'End' }, ], edges: [ @@ -325,9 +325,9 @@ describe('#3863 — a handled failure does not trigger flow-level retry', () => errorHandling: { strategy: 'retry', maxRetries: 3, backoffMs: 1 }, nodes: [ { id: 'start', type: 'start', label: 'Start' }, - { id: 'upstream', type: 'script' as any, label: 'Upstream' }, - { id: 'risky', type: 'script' as any, label: 'Risky' }, - { id: 'handler', type: 'script' as any, label: 'Handler' }, + { id: 'upstream', type: 'script' as any, label: 'Upstream', config: { function: 'noop' } }, + { id: 'risky', type: 'script' as any, label: 'Risky', config: { function: 'noop' } }, + { id: 'handler', type: 'script' as any, label: 'Handler', config: { function: 'noop' } }, { id: 'end', type: 'end', label: 'End' }, ], edges: [ diff --git a/packages/services/service-automation/src/flow-activation-ledger.test.ts b/packages/services/service-automation/src/flow-activation-ledger.test.ts index 6266fbca347..367b410b010 100644 --- a/packages/services/service-automation/src/flow-activation-ledger.test.ts +++ b/packages/services/service-automation/src/flow-activation-ledger.test.ts @@ -304,7 +304,8 @@ describe('ADR-0126 §7.3 — disabling a flow is refused while packaged flows ca ...packagedFlow(name), nodes: [ { id: 'start', type: 'start', label: 'Start', config: {} }, - { id: 'call', type: nodeType, label: 'Call', config: { flowName: target } }, + // A `map` carries the `collection` its executor contract requires (#20316). + { id: 'call', type: nodeType, label: 'Call', config: { flowName: target, ...(nodeType === 'map' ? { collection: [] } : {}) } }, { id: 'end', type: 'end', label: 'End' }, ], edges: [ diff --git a/packages/services/service-automation/src/flow-retry-attempt-count.test.ts b/packages/services/service-automation/src/flow-retry-attempt-count.test.ts index a76fd9338e3..69ac84dd576 100644 --- a/packages/services/service-automation/src/flow-retry-attempt-count.test.ts +++ b/packages/services/service-automation/src/flow-retry-attempt-count.test.ts @@ -49,7 +49,7 @@ function failingFlowEngine(errorHandling: unknown) { errorHandling, nodes: [ { id: 'start', type: 'start', label: 'Start' }, - { id: 'work', type: 'script' as any, label: 'Work' }, + { id: 'work', type: 'script' as any, label: 'Work', config: { function: 'noop' } }, { id: 'end', type: 'end', label: 'End' }, ], edges: [ diff --git a/packages/services/service-automation/src/guard-refusal-inventory.test.ts b/packages/services/service-automation/src/guard-refusal-inventory.test.ts index 893ed268794..6368f76e450 100644 --- a/packages/services/service-automation/src/guard-refusal-inventory.test.ts +++ b/packages/services/service-automation/src/guard-refusal-inventory.test.ts @@ -62,7 +62,7 @@ function flowWithHandler(name: string, node: Record) { nodes: [ { id: 'start', type: 'start' as const, label: 'Start' }, { id: 'op', label: 'Op', ...node }, - { id: 'handler', type: 'script' as any, label: 'Handler' }, + { id: 'handler', type: 'script' as any, label: 'Handler', config: { function: 'noop' } }, { id: 'end', type: 'end' as const, label: 'End' }, ], edges: [ @@ -78,8 +78,15 @@ function flowWithHandler(name: string, node: Record) { * Every guard that must survive a declared fault edge. `node` is the operative * node; `expect` is a fragment of the refusal the run must fail with, so a guard * that starts failing for a DIFFERENT reason does not pass vacuously. + * + * `strip` (#20316): a key the node's executor contract requires is refused + * ABSENT at the build doors now — `registerFlow` parses first — so a flow + * missing it cannot register. The executor's own refusal is still the one this + * inventory classifies, so those rows register the node WHOLE and remove the + * key from the stored flow before the run: the shape an executor meets when a + * config reaches it past the doors. */ -const GUARDS: Array<{ name: string; why: string; node: Record; expect: string }> = [ +const GUARDS: Array<{ name: string; why: string; node: Record; expect: string; strip?: string }> = [ // Since #4277 a missing REQUIRED key is refused by the executor's contract // parse (parse-config.ts) before the hand-written guard runs, so those // entries pin the parse refusal's fragment. The classification is the @@ -88,25 +95,29 @@ const GUARDS: Array<{ name: string; why: string; node: Record; { name: 'get_record without objectName', why: 'a required config key — no run can supply it', - node: { type: 'get_record', config: { filter: { id: 'x' } } }, + node: { type: 'get_record', config: { objectName: 'deal', filter: { id: 'x' } } }, + strip: 'objectName', expect: 'does not satisfy the get_record contract', }, { name: 'create_record without objectName', why: 'a required config key', - node: { type: 'create_record', config: { fields: { a: 1 } } }, + node: { type: 'create_record', config: { objectName: 'deal', fields: { a: 1 } } }, + strip: 'objectName', expect: 'does not satisfy the create_record contract', }, { name: 'update_record without objectName', why: 'a required config key', - node: { type: 'update_record', config: { fields: { a: 1 } } }, + node: { type: 'update_record', config: { objectName: 'deal', fields: { a: 1 } } }, + strip: 'objectName', expect: 'does not satisfy the update_record contract', }, { name: 'delete_record without objectName', why: 'a required config key', - node: { type: 'delete_record', config: { filter: { id: 'x' } } }, + node: { type: 'delete_record', config: { objectName: 'deal', filter: { id: 'x' } } }, + strip: 'objectName', expect: 'does not satisfy the delete_record contract', }, { @@ -133,13 +144,15 @@ const GUARDS: Array<{ name: string; why: string; node: Record; { name: 'http without url', why: 'a required config key', - node: { type: 'http', config: { method: 'GET' } }, + node: { type: 'http', config: { url: 'https://example.invalid/x', method: 'GET' } }, + strip: 'url', expect: 'does not satisfy the http contract', }, { name: 'subflow without flowName', why: 'a required config key', - node: { type: 'subflow', config: {} }, + node: { type: 'subflow', config: { flowName: 'child_flow' } }, + strip: 'flowName', // #4343 moved this from a hand-written `refuseNode` to the contract // parse, like the CRUD entries above. Same classification, same node — // only the message is now derived from `SubflowConfigSchema`. @@ -148,7 +161,8 @@ const GUARDS: Array<{ name: string; why: string; node: Record; { name: 'map without flowName', why: 'a required config key — the per-item subflow', - node: { type: 'map', config: { collection: [] } }, + node: { type: 'map', config: { collection: [], flowName: 'child_flow' } }, + strip: 'flowName', expect: 'flowName', }, { @@ -194,7 +208,7 @@ describe('#3863 — the guard inventory stays un-routable', () => { it.each(GUARDS.map((g, i) => ({ ...g, i })))( '$name stays fatal with a fault edge ($why)', - async ({ node, expect: fragment, i }) => { + async ({ node, expect: fragment, i, strip }) => { let handlerRan = false; engine.registerNodeExecutor({ type: 'script', @@ -204,7 +218,8 @@ describe('#3863 — the guard inventory stays un-routable', () => { }, }); const flowName = `guard_case_${i}`; - engine.registerFlow(flowName, flowWithHandler(flowName, node) as any); + const stored = engine.registerFlow(flowName, flowWithHandler(flowName, node) as any); + if (strip) delete (stored.nodes.find((n) => n.id === 'op')!.config as Record)[strip]; const result = await engine.execute(flowName, { record: { id: 'r1', owner: 'usr_7' } } as any); @@ -237,7 +252,7 @@ describe('#3863 — runtime failures stay routable', () => { return { success: true }; }, }); - engine.registerFlow('runtime_ok', flowWithHandler('runtime_ok', { type: 'script' }) as any); + engine.registerFlow('runtime_ok', flowWithHandler('runtime_ok', { type: 'script', config: { function: 'noop' } }) as any); const result = await engine.execute('runtime_ok'); expect(result.success).toBe(true); @@ -254,7 +269,7 @@ describe('#3863 — runtime failures stay routable', () => { return { success: true }; }, }); - engine.registerFlow('throw_ok', flowWithHandler('throw_ok', { type: 'script' }) as any); + engine.registerFlow('throw_ok', flowWithHandler('throw_ok', { type: 'script', config: { function: 'noop' } }) as any); const result = await engine.execute('throw_ok'); expect(result.success).toBe(true); diff --git a/packages/services/service-automation/src/input-schema-retry-parity.test.ts b/packages/services/service-automation/src/input-schema-retry-parity.test.ts index 7dfb86c0578..ba0023438b5 100644 --- a/packages/services/service-automation/src/input-schema-retry-parity.test.ts +++ b/packages/services/service-automation/src/input-schema-retry-parity.test.ts @@ -74,7 +74,9 @@ function countingFlowEngine(opts: { id: 'work', type: 'script' as any, label: 'Work', - config: opts.config, + // `function` is the key the script executor contract requires; + // the flow parse refuses a script node without it (#20316). + config: { function: 'noop', ...opts.config }, inputSchema: opts.inputSchema, }, { id: 'end', type: 'end', label: 'End' }, diff --git a/packages/spec/src/automation/flow-node-config-refusals.ts b/packages/spec/src/automation/flow-node-config-refusals.ts index 63a26c991d6..bd1d620a25b 100644 --- a/packages/spec/src/automation/flow-node-config-refusals.ts +++ b/packages/spec/src/automation/flow-node-config-refusals.ts @@ -19,6 +19,7 @@ import { NON_BLANK_STRING } from '../shared/refinement-projection'; import { FLOW_REGION_SLOTS_BY_TYPE } from './region-slots'; +import { FLOW_NODE_EXPRESSION_PATHS } from './flow-node-expression-paths'; import type { FlowNodeConfigRefusal, FlowSlotRefusalParams, NodeConfigValueKind } from './flow-node-expression-paths'; // The executor contracts. Read only inside `getBuiltinNodeConfigContracts`, // never at module load: `control-flow.zod.ts` sits in the flow-schema import @@ -160,6 +161,22 @@ function insideRegion(nodeType: string, path: ReadonlyArray): boole return (FLOW_REGION_SLOTS_BY_TYPE.get(nodeType) ?? []).some((slot) => slot.key === path[0]); } +/** + * Does an issue path descend INTO a ledger `value` slot (`fields.total.source` + * under `fields.*`)? Such a slot holds an authored VALUE — a literal, a + * `{token}` template, or an expression envelope — and a key missing inside it + * is a malformed value, judged at `registerFlow` and `objectstack validate` by + * the value-envelope pass with its own refusal, never a config key left out. + */ +function insideValueSlot(nodeType: string, path: ReadonlyArray): boolean { + return FLOW_NODE_EXPRESSION_PATHS.some((entry) => { + if (entry.nodeType !== nodeType || entry.role !== 'value') return false; + const segments = entry.path.split('.'); + if (path.length <= segments.length) return false; + return segments.every((segment, i) => segment === '*' || segment === path[i]); + }); +} + /** The refusal for a key a node's executor contract requires. */ function nodeConfigKeyMissingMessage(nodeType: string, key: string): string { return ( @@ -186,7 +203,10 @@ function nodeConfigKeyMissingMessage(nodeType: string, key: string): string { * authored. That keeps the judge to one question — "would the run refuse this * node for a key it leaves out?" — and leaves every other contract finding * (a present value of the wrong type, an undeclared key) where it lives - * today. Issues inside an ADR-0031 region are the region's own and skipped. + * today. Issues inside an ADR-0031 region are the region's own and skipped, + * and so are issues inside a ledger `value` slot (`fields.*`, + * `assignments.*`): a key missing inside an authored value is a malformed + * value, refused by the value-envelope pass, not a config key left out. * * - A key the contract simply requires → `node-config-key-missing`, whose * message names the key and the node type. @@ -237,6 +257,7 @@ export function flowNodeConfigRefusals(nodeType: string, config: unknown): FlowN for (const issue of result.error?.issues ?? []) { if (issue.path.length === 0) continue; if (insideRegion(nodeType, issue.path)) continue; + if (insideValueSlot(nodeType, issue.path)) continue; if (!absentAt(authored, issue.path)) continue; const key = ledgerPathOf(issue.path); if (seen.has(key)) continue; diff --git a/packages/spec/src/automation/flow-node-config-required.test.ts b/packages/spec/src/automation/flow-node-config-required.test.ts index 5c10479333a..4979d905d54 100644 --- a/packages/spec/src/automation/flow-node-config-required.test.ts +++ b/packages/spec/src/automation/flow-node-config-required.test.ts @@ -165,6 +165,14 @@ describe('FlowSchema.parse refuses a key the node\'s executor contract requires, expect(issuesOf(flowWith(node('script', { function: '' })))).toEqual([]); }); + it('a key missing INSIDE an authored value is not a config key left out — the value-envelope pass owns it', () => { + // `fields.*` is a ledger `value` slot: `{ dialect: 'cel' }` with no + // `source` is a malformed envelope, refused at `registerFlow` and + // `objectstack validate` by that pass, with its own message. + expect(flowNodeConfigRefusals('create_record', { objectName: 'task', fields: { total: { dialect: 'cel' } } })).toEqual([]); + expect(flowNodeConfigRefusals('update_record', { objectName: 'task', filter: { id: '1' }, fields: { total: { dialect: 'cel' } } })).toEqual([]); + }); + it('an undeclared key is not this rule\'s finding — no key-set closure', () => { expect(issuesOf(flowWith(node('http', { url: 'https://example.com', zzz_undeclared: 1 })))).toEqual([]); }); From 60e57913d472825b8d0306de041da3b7a6b73323 Mon Sep 17 00:00:00 2001 From: Claude Date: Mon, 28 Sep 2026 06:15:16 +0000 Subject: [PATCH 6/8] wip: lint callable check keeps a script's function; complete runtime / types / verify fixtures Claude-Session: https://claude.ai/code/session_01QcAS3qiYYZNezaxZxaUdMV Co-Authored-By: Claude --- packages/lint/src/validate-expressions.test.ts | 10 +++------- packages/lint/src/validate-expressions.ts | 13 ++++++++----- .../src/domains/automation-flow-clone.test.ts | 2 +- .../automation-put-post-error-parity.test.ts | 14 +++++++++----- .../automation-register-error-class.test.ts | 14 +++++++++----- packages/types/src/validation-failure.test.ts | 10 +++++++--- .../automation-trigger-terminal-messages.test.ts | 2 +- 7 files changed, 38 insertions(+), 27 deletions(-) diff --git a/packages/lint/src/validate-expressions.test.ts b/packages/lint/src/validate-expressions.test.ts index 9186919d1a8..a05d8c01703 100644 --- a/packages/lint/src/validate-expressions.test.ts +++ b/packages/lint/src/validate-expressions.test.ts @@ -4497,7 +4497,7 @@ describe('node config an executor requires (#20316)', () => { ['map', { collection: '{rows}', flowName: 'child_flow' }, 'collection'], ['get_record', { objectName: 'account' }, 'objectName'], ['http', { url: 'https://example.com/hook' }, 'url'], - ['script', { function: 'recalc_totals' }, 'function'], + ['subflow', { flowName: 'child_flow' }, 'flowName'], ] as Array<[string, Record, string]>)('%s without `%s` is refused; with it, nothing is', (type, whole, key) => { expect(errorsOf(stackWith({ type, config: whole }))).toHaveLength(0); const authored = { ...whole }; @@ -4522,14 +4522,10 @@ describe('node config an executor requires (#20316)', () => { expect(found.map((i) => i.where)).toEqual(["flow 'config_flow' · node 'n' (decision) config.conditions[0]"]); }); - it('a `script` with no `function` is ONE finding — the judge\'s, not also the callable check\'s', () => { + it('a `script` with no `function` is ONE finding, the callable check\'s — it reads the pre-conversion spellings this pass may be handed', () => { const found = errorsOf(stackWith({ type: 'script', config: {} })); - expect(found.map((i) => i.where)).toEqual(["flow 'config_flow' · node 'n' (script) config.function"]); - }); - - it('CONTROL — a `script` whose `function` is present but blank keeps the callable check\'s finding', () => { - const found = errorsOf(stackWith({ type: 'script', config: { function: ' ' } })); expect(found.map((i) => i.where)).toEqual(["flow 'config_flow' · node 'n' (script) callable"]); + expect(errorsOf(stackWith({ type: 'script', config: { functionName: 'recalc_totals' } }))).toHaveLength(0); }); }); diff --git a/packages/lint/src/validate-expressions.ts b/packages/lint/src/validate-expressions.ts index e1c22327f7d..756324c562d 100644 --- a/packages/lint/src/validate-expressions.ts +++ b/packages/lint/src/validate-expressions.ts @@ -1628,7 +1628,13 @@ export function validateStackExpressions(stack: AnyRec): ExprIssue[] { // handed to `validateStackExpressions` without a parse in front of it // is held to the same bar. `error`: the flow would register and then // refuse — or, for a branch with no label, misroute — every run. - const configRefusals = flowNodeConfigRefusals(nodeType, node.config); + const configRefusals = flowNodeConfigRefusals(nodeType, node.config) + // A `script`'s `function` stays the callable check's below (#1870, + // #4343): this pass may be handed a pre-conversion source, and that + // check reads what such a source spells — the `functionName` alias, + // the retired dispatch keys — and names each, where the judge would + // only see `function` absent. + .filter((r) => !(nodeType === 'script' && r.path === 'function')); for (const refusal of configRefusals) { issues.push({ where: `${at} · node '${node.id}' (${nodeType}) config.${refusal.path}`, @@ -1744,10 +1750,7 @@ export function validateStackExpressions(stack: AnyRec): ExprIssue[] { + 'sources; apply them by hand.', source: JSON.stringify({ id: node.id, type: node.type, config: cfg }), }); - } else if (!fn && !configRefusals.some((r) => r.path === 'function')) { - // [#20316] An ABSENT `function` is the contract judge's finding - // above (one finding, not two); this arm keeps the shape that judge - // leaves alone — a `function` that is present and blank. + } else if (!fn) { issues.push({ where: `${at} · node '${node.id}' (script) callable`, message: diff --git a/packages/runtime/src/domains/automation-flow-clone.test.ts b/packages/runtime/src/domains/automation-flow-clone.test.ts index 8db5f6aaea8..e3be8f3b33a 100644 --- a/packages/runtime/src/domains/automation-flow-clone.test.ts +++ b/packages/runtime/src/domains/automation-flow-clone.test.ts @@ -100,7 +100,7 @@ function packagedExemplar(): Record { condition: 'record.stage == "negotiation"', }, }, - { id: 'notify', type: 'notify', label: 'Notify manager', config: { channel: 'email', to: '{record.manager_email}' } }, + { id: 'notify', type: 'notify', label: 'Notify manager', config: { channel: 'email', to: '{record.manager_email}', title: 'Deal in negotiation' } }, { id: 'end', type: 'end', label: 'End' }, ], edges: [ diff --git a/packages/runtime/src/domains/automation-put-post-error-parity.test.ts b/packages/runtime/src/domains/automation-put-post-error-parity.test.ts index 9c85dc81c73..e2be7d6ba7f 100644 --- a/packages/runtime/src/domains/automation-put-post-error-parity.test.ts +++ b/packages/runtime/src/domains/automation-put-post-error-parity.test.ts @@ -127,12 +127,16 @@ function makeDispatcher() { */ const CTX = { request: {}, executionContext: { userId: 'user_1', systemPermissions: ['manage_metadata'] } } as any; -/** A definition that is legal at every gate the fake runs. */ +/** + * A definition that is legal at every gate the fake runs. The notify node + * carries the `recipients` and `title` its executor contract requires — the + * flow parse refuses a node that leaves them out (#20316). + */ const WELL_FORMED = { name: 'welcome_flow', label: 'Welcome', type: 'autolaunched', - nodes: [{ id: 'n', type: 'notify', label: 'Notify', config: { message: 'hi' } }], + nodes: [{ id: 'n', type: 'notify', label: 'Notify', config: { recipients: ['user_1'], title: 'Welcome', message: 'hi' } }], edges: [], }; @@ -141,12 +145,12 @@ const BAD_BODIES = { /** 1 — a node with no `label` (`FlowSchema.parse`). */ missingNodeLabel: { ...WELL_FORMED, - nodes: [{ id: 'n', type: 'notify', config: { message: 'hi' } }], + nodes: [{ id: 'n', type: 'notify', config: { recipients: ['user_1'], title: 'Welcome', message: 'hi' } }], }, /** 2 — a node key the schema does not declare (`unrecognized_keys`). */ unknownNodeKey: { ...WELL_FORMED, - nodes: [{ id: 'n', type: 'notify', label: 'Notify', next: 'other' }], + nodes: [{ id: 'n', type: 'notify', label: 'Notify', config: { recipients: ['user_1'], title: 'Welcome', message: 'hi' }, next: 'other' }], }, /** 3 — a `try_catch` whose `try` region is an array, not a region object. */ malformedRegion: { @@ -161,7 +165,7 @@ const BAD_BODIES = { ...WELL_FORMED, nodes: [{ id: 'n', type: 'notify', label: 'Notify', - config: { message: 'hi', totallyBogusKey: 'oops' }, + config: { recipients: ['user_1'], title: 'Welcome', message: 'hi', totallyBogusKey: 'oops' }, }], }, } as const; diff --git a/packages/runtime/src/domains/automation-register-error-class.test.ts b/packages/runtime/src/domains/automation-register-error-class.test.ts index a9ab36e23e2..15d05037bd6 100644 --- a/packages/runtime/src/domains/automation-register-error-class.test.ts +++ b/packages/runtime/src/domains/automation-register-error-class.test.ts @@ -159,12 +159,16 @@ function makeDispatcher(options?: { registerFlow?: (name: string, definition: un */ const CTX = { request: {}, executionContext: { userId: 'user_1', systemPermissions: ['manage_metadata'] } } as any; -/** A definition that is legal at every gate the fake runs. */ +/** + * A definition that is legal at every gate the fake runs. The notify node + * carries the `recipients` and `title` its executor contract requires — the + * flow parse refuses a node that leaves them out (#20316). + */ const WELL_FORMED = { name: 'welcome_flow', label: 'Welcome', type: 'autolaunched', - nodes: [{ id: 'n', type: 'notify', label: 'Notify', config: { message: 'hi' } }], + nodes: [{ id: 'n', type: 'notify', label: 'Notify', config: { recipients: ['user_1'], title: 'Welcome', message: 'hi' } }], edges: [], }; @@ -173,12 +177,12 @@ const BAD_BODIES = { /** 1 — a node with no `label`. */ missingNodeLabel: { ...WELL_FORMED, - nodes: [{ id: 'n', type: 'notify', config: { message: 'hi' } }], + nodes: [{ id: 'n', type: 'notify', config: { recipients: ['user_1'], title: 'Welcome', message: 'hi' } }], }, /** 2 — a node key the schema does not declare. */ unknownNodeKey: { ...WELL_FORMED, - nodes: [{ id: 'n', type: 'notify', label: 'Notify', next: 'other' }], + nodes: [{ id: 'n', type: 'notify', label: 'Notify', config: { recipients: ['user_1'], title: 'Welcome', message: 'hi' }, next: 'other' }], }, /** 3 — a `try_catch` whose `try` region is an array, not a region object. */ malformedRegion: { @@ -193,7 +197,7 @@ const BAD_BODIES = { ...WELL_FORMED, nodes: [{ id: 'n', type: 'notify', label: 'Notify', - config: { message: 'hi', totallyBogusKey: 'oops' }, + config: { recipients: ['user_1'], title: 'Welcome', message: 'hi', totallyBogusKey: 'oops' }, }], }, } as const; diff --git a/packages/types/src/validation-failure.test.ts b/packages/types/src/validation-failure.test.ts index 16cd3d5254a..5c09a0be42b 100644 --- a/packages/types/src/validation-failure.test.ts +++ b/packages/types/src/validation-failure.test.ts @@ -32,12 +32,16 @@ function issuesOf(schema: { safeParse: (v: unknown) => any }, value: unknown) { return r.error.issues; } -/** A flow definition that parses clean — fixtures below are one edit away. */ +/** + * A flow definition that parses clean — fixtures below are one edit away. The + * notify node carries the `recipients` and `title` its executor contract + * requires; the flow parse refuses a node that leaves them out (#20316). + */ const WELL_FORMED_FLOW = { name: 'welcome_flow', label: 'Welcome', type: 'autolaunched', - nodes: [{ id: 'n', type: 'notify', label: 'Notify', config: { message: 'hi' } }], + nodes: [{ id: 'n', type: 'notify', label: 'Notify', config: { recipients: ['user_1'], title: 'Welcome', message: 'hi' } }], edges: [], }; @@ -92,7 +96,7 @@ describe('fieldsFromZodIssues — ADR-0114 D3 catalog codes, not Zod codes (#812 }); it('the optional input upgrades a missing required property to required', () => { - const bad = { ...WELL_FORMED_FLOW, nodes: [{ id: 'n', type: 'notify', config: { message: 'hi' } }] }; + const bad = { ...WELL_FORMED_FLOW, nodes: [{ id: 'n', type: 'notify', config: { recipients: ['user_1'], title: 'Welcome', message: 'hi' } }] }; const issues = issuesOf(FlowSchema, bad); // Without the input — every caller today — the D3 degradation: still a diff --git a/packages/verify/src/automation-trigger-terminal-messages.test.ts b/packages/verify/src/automation-trigger-terminal-messages.test.ts index ee20bffe363..afde252e1c1 100644 --- a/packages/verify/src/automation-trigger-terminal-messages.test.ts +++ b/packages/verify/src/automation-trigger-terminal-messages.test.ts @@ -86,7 +86,7 @@ function bootFlow(opts: { fails: boolean; successMessage?: string; errorMessage? ...(opts.errorMessage !== undefined ? { errorMessage: opts.errorMessage } : {}), nodes: [ { id: 'start', type: 'start', label: 'Start' }, - { id: 'work', type: 'script', label: 'Work' }, + { id: 'work', type: 'script', label: 'Work', config: { function: 'noop' } }, { id: 'end', type: 'end', label: 'End' }, ], edges: [ From 7f3b9b6742c5434cff7c2afbb5be001cf9d3d3de Mon Sep 17 00:00:00 2001 From: Claude Date: Mon, 28 Sep 2026 08:20:05 +0000 Subject: [PATCH 7/8] wip: lint region fixtures and receiver excusal; runtime fake notify declares title Claude-Session: https://claude.ai/code/session_01QcAS3qiYYZNezaxZxaUdMV Co-Authored-By: Claude --- packages/lint/src/validate-expressions.test.ts | 9 +++++++-- packages/lint/src/validate-expressions.ts | 10 +++++----- .../domains/automation-put-post-error-parity.test.ts | 4 +++- .../domains/automation-register-error-class.test.ts | 4 +++- 4 files changed, 18 insertions(+), 9 deletions(-) diff --git a/packages/lint/src/validate-expressions.test.ts b/packages/lint/src/validate-expressions.test.ts index a05d8c01703..5c3f2a232c5 100644 --- a/packages/lint/src/validate-expressions.test.ts +++ b/packages/lint/src/validate-expressions.test.ts @@ -2292,7 +2292,7 @@ describe('validateStackExpressions (ADR-0032 build-time)', () => { const badRegion = () => ({ nodes: [ { id: 'gate', type: 'decision', config: { condition: '{record.rating} >= 4' } }, - { id: 'act', type: 'update_record' }, + { id: 'act', type: 'update_record', config: { objectName: 'crm_lead' } }, ], edges: [{ id: 'b1', source: 'gate', target: 'act', condition: '{record.status} == "open"' }], }); @@ -2342,7 +2342,7 @@ describe('validateStackExpressions (ADR-0032 build-time)', () => { body: { nodes: [ { id: 'gate', type: 'decision', config: { condition: 'record.rating >= 4' } }, - { id: 'act', type: 'update_record' }, + { id: 'act', type: 'update_record', config: { objectName: 'crm_lead' } }, ], edges: [{ id: 'b1', source: 'gate', target: 'act', condition: 'record.status == "open"' }], }, @@ -3011,6 +3011,11 @@ describe('validateStackExpressions — reads only keys the spec declares (meta-t // are `success` / `error`, SafeParseResult's own, so that excuse covers // both locals for one reason and masks no metadata read either. 'blankRefusal', + // [#20316] The spec's node-config judge, one refusal at a time. Its keys + // are that helper's own `{ code, params, message, source, path }` — + // never metadata keys — and it is named to stay clear of the `message` / + // `source` receivers for the reason the two entries above record. + 'configRefusal', // [#14089] NOT a receiver at all — the tail of the `'./flow-variable-scope.js'` // import specifier, which this scan cannot tell from `scope.j…`. The two // entries above it in this set (`fields`, `guards`) are the same artefact diff --git a/packages/lint/src/validate-expressions.ts b/packages/lint/src/validate-expressions.ts index 756324c562d..0fb0236cc02 100644 --- a/packages/lint/src/validate-expressions.ts +++ b/packages/lint/src/validate-expressions.ts @@ -1634,12 +1634,12 @@ export function validateStackExpressions(stack: AnyRec): ExprIssue[] { // check reads what such a source spells — the `functionName` alias, // the retired dispatch keys — and names each, where the judge would // only see `function` absent. - .filter((r) => !(nodeType === 'script' && r.path === 'function')); - for (const refusal of configRefusals) { + .filter((configRefusal) => !(nodeType === 'script' && configRefusal.path === 'function')); + for (const configRefusal of configRefusals) { issues.push({ - where: `${at} · node '${node.id}' (${nodeType}) config.${refusal.path}`, - message: refusal.message, - source: refusal.source, + where: `${at} · node '${node.id}' (${nodeType}) config.${configRefusal.path}`, + message: configRefusal.message, + source: configRefusal.source, severity: 'error', }); } diff --git a/packages/runtime/src/domains/automation-put-post-error-parity.test.ts b/packages/runtime/src/domains/automation-put-post-error-parity.test.ts index e2be7d6ba7f..7296e9e65dc 100644 --- a/packages/runtime/src/domains/automation-put-post-error-parity.test.ts +++ b/packages/runtime/src/domains/automation-put-post-error-parity.test.ts @@ -45,7 +45,9 @@ import { FlowSchema, validateControlFlow } from '@objectstack/spec/automation'; import { HttpDispatcher } from '../http-dispatcher.js'; /** Config keys the fake's `notify` descriptor declares (the #4277 legal set). */ -const NOTIFY_DECLARED_CONFIG_KEYS = ['message', 'recipients', 'channel']; +// `title` joined the set when the flow parse began refusing a notify node with +// neither `title` nor `template` (#20316) — the real notify descriptor declares it. +const NOTIFY_DECLARED_CONFIG_KEYS = ['title', 'message', 'recipients', 'channel']; /** * The #4277 refusal, reproduced from `service-automation/src/engine.ts` diff --git a/packages/runtime/src/domains/automation-register-error-class.test.ts b/packages/runtime/src/domains/automation-register-error-class.test.ts index 15d05037bd6..b2cf8970e40 100644 --- a/packages/runtime/src/domains/automation-register-error-class.test.ts +++ b/packages/runtime/src/domains/automation-register-error-class.test.ts @@ -74,7 +74,9 @@ import { FlowSchema, validateControlFlow } from '@objectstack/spec/automation'; import { HttpDispatcher } from '../http-dispatcher.js'; /** Config keys the fake's `notify` descriptor declares (the #4277 legal set). */ -const NOTIFY_DECLARED_CONFIG_KEYS = ['message', 'recipients', 'channel']; +// `title` joined the set when the flow parse began refusing a notify node with +// neither `title` nor `template` (#20316) — the real notify descriptor declares it. +const NOTIFY_DECLARED_CONFIG_KEYS = ['title', 'message', 'recipients', 'channel']; /** * The #4277 refusal, reproduced from `service-automation/src/engine.ts` From 8171d8537e9efe68310733e9fa4cc9a51f5a2f4a Mon Sep 17 00:00:00 2001 From: Claude Date: Mon, 28 Sep 2026 09:01:38 +0000 Subject: [PATCH 8/8] test(lint): name each node-config census row by its type and key Claude-Session: https://claude.ai/code/session_01QcAS3qiYYZNezaxZxaUdMV Co-Authored-By: Claude --- packages/lint/src/validate-expressions.test.ts | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/packages/lint/src/validate-expressions.test.ts b/packages/lint/src/validate-expressions.test.ts index 5c3f2a232c5..de440d9319b 100644 --- a/packages/lint/src/validate-expressions.test.ts +++ b/packages/lint/src/validate-expressions.test.ts @@ -4498,12 +4498,12 @@ describe('node config an executor requires (#20316)', () => { validateStackExpressions(stack as never).filter((i) => (i.severity ?? 'error') === 'error'); it.each([ - ['loop', { collection: '{rows}', body: { nodes: [{ id: 'b', type: 'assignment' }], edges: [] } }, 'collection'], - ['map', { collection: '{rows}', flowName: 'child_flow' }, 'collection'], - ['get_record', { objectName: 'account' }, 'objectName'], - ['http', { url: 'https://example.com/hook' }, 'url'], - ['subflow', { flowName: 'child_flow' }, 'flowName'], - ] as Array<[string, Record, string]>)('%s without `%s` is refused; with it, nothing is', (type, whole, key) => { + { type: 'loop', whole: { collection: '{rows}', body: { nodes: [{ id: 'b', type: 'assignment' }], edges: [] } }, key: 'collection' }, + { type: 'map', whole: { collection: '{rows}', flowName: 'child_flow' }, key: 'collection' }, + { type: 'get_record', whole: { objectName: 'account' }, key: 'objectName' }, + { type: 'http', whole: { url: 'https://example.com/hook' }, key: 'url' }, + { type: 'subflow', whole: { flowName: 'child_flow' }, key: 'flowName' }, + ] as Array<{ type: string; whole: Record; key: string }>)('$type without `$key` is refused; with it, nothing is', ({ type, whole, key }) => { expect(errorsOf(stackWith({ type, config: whole }))).toHaveLength(0); const authored = { ...whole }; delete authored[key];