From 63e239cdbfaf0cff5fdabb80be6764fd601b3601 Mon Sep 17 00:00:00 2001 From: Claude Date: Thu, 8 Oct 2026 12:53:08 +0000 Subject: [PATCH 01/10] feat(spec,service-automation): the spec judges an undeclared key on every builtin but try_catch; the descriptor walk stands aside (wip) Claude-Session: https://claude.ai/code/session_01DhTqaEHqPVSVnAkjG3jywn Co-authored-by: Claude --- .../services/service-automation/src/engine.ts | 35 +++- .../automation/flow-node-config-refusals.ts | 195 ++++++++++++------ packages/spec/src/automation/flow.zod.ts | 35 ++-- ...tin-node-config-undeclared-keys-refused.ts | 83 ++++++++ packages/spec/src/migrations/registry.ts | 95 +++++++++ 5 files changed, 367 insertions(+), 76 deletions(-) create mode 100644 packages/spec/src/migrations/entries/semantic/18.flow-builtin-node-config-undeclared-keys-refused.ts diff --git a/packages/services/service-automation/src/engine.ts b/packages/services/service-automation/src/engine.ts index ab35572c826..fb1b2f8bfeb 100644 --- a/packages/services/service-automation/src/engine.ts +++ b/packages/services/service-automation/src/engine.ts @@ -48,6 +48,11 @@ import { FlowValueSlotSchema, VALUE_ENVELOPE_REFUSAL } from '@objectstack/spec/a // judge, shared with `objectstack validate` (`@objectstack/lint` calls the same // function on the same config), so build and registration give one verdict. import { flowNodeValueTemplateRefusals } from '@objectstack/spec/automation'; +// [#21982] Which node types the spec judges an undeclared config key for — the +// ONE judge of a builtin's undeclared key, met here through `FlowSchema.parse`. +// `validateNodeConfigKeys` asks it and stands aside for every type it names, +// so no node type has two judges. +import { builtinNodeConfigKeysJudged } from '@objectstack/spec/automation'; // [#17322] The EVALUATED-slot rule, IMPORTED rather than re-derived. It is the // rule `FlowEdgeSchema.condition` already composes since #15807, so a node's // `config.condition` — which no schema stands in front of — is held to the same @@ -168,6 +173,16 @@ interface ConfigSchemaNode { * history is noise. The generic rejection already carries the path, the * did-you-mean and the declared set; an entry here adds the *mechanism* the * author was reaching for. + * + * ⚠️ [#21982] UNREAD today: every type keyed below (`create_record`, + * `update_record`, `delete_record`, `screen`) is one the spec judges + * ({@link builtinNodeConfigKeysJudged}), so {@link + * AutomationEngine.validateNodeConfigKeys} stands aside for it and never + * reaches this table. Each entry's prescription is carried by that type's + * executor contract (`builtin-node-config.zod.ts`, its strictObject + * `guidance`), which is what an author now meets, at every door. Kept only + * until the comments in that spec module stop naming this table; a plugin + * node type would add its own entry here. */ /** * The bulk-intent spellings, shared by `update_record` and `delete_record` @@ -4386,7 +4401,9 @@ export class AutomationEngine implements IAutomationService { // silence). Hard-fail with per-key prescriptions: see // validateNodeConfigKeys for why the #4045 reconciliation made this // safe, and for the deliberate exemptions (`assignment`, schemaless - // types, keyValue maps). + // types, keyValue maps). [#21982] Only for the types the spec does not + // judge — `try_catch` and plugin node types; every other builtin's + // undeclared key was refused by the `FlowSchema.parse` above. this.validateNodeConfigKeys(name, parsed); // #15429 — parse every `decision` node's config against the spec's @@ -10273,6 +10290,19 @@ export class AutomationEngine implements IAutomationService { * keys with documented history — a per-key tombstone from * {@link FLOW_NODE_UNKNOWN_KEY_GUIDANCE}. * + * **[#21982] One judge per node type.** For every builtin the spec judges + * ({@link builtinNodeConfigKeysJudged} — every builtin with an executor + * contract but `try_catch`), the spec's key arm is the judge, at every + * door: `FlowSchema.parse` above already refused such a key, anchored at + * `nodes.N.config.` in the contract's own words, before this walk + * runs. So this walk stands aside for those types and keeps the rest: + * `try_catch`, whose descriptor closes `retry` where its contract's `retry` + * strips an unknown key, and every PLUGIN node type, whose contract the + * spec does not declare and whose descriptor `configSchema` is the only + * declaration there is. The per-key tombstones below + * ({@link FLOW_NODE_UNKNOWN_KEY_GUIDANCE}) live on in the contracts' own + * unknown-key prescriptions for the types the spec now judges. + * * Deliberate exemptions, unchanged from the warn era: * - **`assignment` is exempt wholesale**: with no `assignments` wrapper * its top-level config keys ARE the author's variable names @@ -10305,6 +10335,9 @@ export class AutomationEngine implements IAutomationService { // `assignment` config keys are the author's variable names — see the // exemption note above. if (node.type === 'assignment') continue; + // [#21982] The spec judged this type's keys in `FlowSchema.parse`: + // one judge per type, never two. + if (builtinNodeConfigKeysJudged(node.type)) continue; const schema = this.actionDescriptors.get(node.type)?.configSchema as ConfigSchemaNode | undefined; if (!schema) continue; this.collectUndeclaredConfigKeys(node, schema, node.config, 'config', scoped); diff --git a/packages/spec/src/automation/flow-node-config-refusals.ts b/packages/spec/src/automation/flow-node-config-refusals.ts index 154b3502de2..8adcb8e2a7d 100644 --- a/packages/spec/src/automation/flow-node-config-refusals.ts +++ b/packages/spec/src/automation/flow-node-config-refusals.ts @@ -15,9 +15,10 @@ * builtin node's executor contract refuses**, where the build can know what * the run will parse — see {@link flowNodeConfigRefusals}' first arm for what * that excludes, and why. And (#21982) **a key a builtin's executor contract - * does not declare, where no other door judges it** — `script` and `subflow`, - * whose descriptors publish no `configSchema` for registration's key check to - * read ({@link builtinKeysJudged}). + * does not declare** — every builtin in the contract map but `try_catch` + * ({@link builtinNodeConfigKeysJudged}), the one judge of a builtin's + * undeclared key at every door: `registerFlow`'s descriptor walk stands aside + * for exactly the types this judges. * * Its refusal codes join the closed flow slot table * (`FLOW_SLOT_REFUSAL_CODES`, `flow-node-expression-paths.ts`); the @@ -47,10 +48,7 @@ import { UpdateRecordConfigSchema, } from './builtin-node-config.zod'; import { HttpConfigSchema, NotifyConfigSchema } from './io-node-config.zod'; -// [#21982] `SCHEMALESS_NODE_CONFIG_SCHEMAS` is the spec's own record of the -// node types that publish no descriptor `configSchema`; read only inside -// `builtinKeysJudged`, like the contracts beside it. -import { SCHEMALESS_NODE_CONFIG_SCHEMAS, ScriptConfigSchema, SubflowConfigSchema } from './schemaless-node-config.zod'; +import { ScriptConfigSchema, SubflowConfigSchema } from './schemaless-node-config.zod'; // [#21850] The one plugin node contract the spec declares. Read only inside // `getDeclaredPluginNodeConfigContracts`, like the executor contracts above. // `approval.zod.ts` imports nothing from `automation/` (zod, the membership-role @@ -79,6 +77,12 @@ export interface BuiltinNodeConfigContract { * 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. + * + * (#21982) It gates presence and values only. Key MEMBERSHIP is judged + * whatever it says, for a type {@link builtinNodeConfigKeysJudged} judges: + * an undeclared key is never read on either path. Registration refused one + * on a body-less `loop` before the build doors judged keys, and it still + * refuses one there. */ readonly parsedWhen?: (config: Readonly>) => boolean; } @@ -338,11 +342,10 @@ interface ContractIssue { * * - key membership — an undeclared key (`unrecognized_keys`) and a tombstoned * one (a `retiredKey()`, an `invalid_type` expecting `never`): an undeclared - * key is the key arm's where no descriptor publishes a `configSchema` - * ({@link builtinKeysJudged}: `script`, `subflow`), and registration's - * everywhere else — it refuses one against the descriptor with its own - * prescriptions; the lint names the retired script keys, and the conversion - * layer rewrites a retired spelling before the two doors that convert first; + * key is the key arm's ({@link builtinNodeConfigKeysJudged}), and on + * `try_catch` registration's, against its descriptor; the lint names the + * retired script keys, and the conversion layer rewrites a retired spelling + * before the two doors that convert first; * - an ADR-0031 region slot, the slot itself included (`try: 5`): a region's * shape is `validateControlFlow`'s, and its nodes are the region walk's; * - a `predicate` or `value` ledger slot, at or inside it @@ -373,25 +376,62 @@ function builtinValueJudged( // ─── The builtin KEY arm (#21982) ───────────────────────────────────── /** - * [#21982] Does the build judge KEY MEMBERSHIP on this builtin's executor - * contract? Only where no door before the run does: a builtin whose descriptor - * publishes no `configSchema`, the spec's own schemaless class - * ({@link SCHEMALESS_NODE_CONFIG_SCHEMAS}). `registerFlow`'s undeclared-key - * walk (`validateNodeConfigKeys`) derives the declared set from that - * descriptor schema, so it skips these types, while their executors parse the - * strict contract and refuse the node on an undeclared key at every run. - * - * Today `script` and `subflow`. `decision` is in the schemaless class but has - * no builtin contract: its executor parses nothing, so an undeclared key fails - * no run. Every other builtin's undeclared key stays registration's, judged - * against its descriptor with that walk's own prescriptions — the spec arm - * does not shadow it, because `registerFlow` parses `FlowSchema` first. - * - * Asked only for a type in {@link getBuiltinNodeConfigContracts}; read on - * first use, like the contracts. + * [#21982] The builtins whose key membership stays `registerFlow`'s, each with + * the measured reason the spec cannot take it: their descriptor's + * `configSchema` closes a key set their executor contract does not, so the + * descriptor walk (`validateNodeConfigKeys`) refuses a key the contract would + * accept, and moving the judge would widen registration. + * + * Today one: `try_catch`. The walk closes `retry` to the five keys its + * descriptor declares, while the contract's `retry` is the shared + * `RetryPolicySchema` — a plain `z.object`, which strips an unknown key rather + * than refusing it. One type, one judge: its top level and its regions stay + * the walk's too, never split between two judges. */ -function builtinKeysJudged(nodeType: string): boolean { - return Object.prototype.hasOwnProperty.call(SCHEMALESS_NODE_CONFIG_SCHEMAS, nodeType); +const BUILTIN_KEYS_JUDGED_AT_REGISTRATION: ReadonlyMap = new Map([ + ['try_catch', 'its contract\'s `retry` (`RetryPolicySchema`) strips an unknown key, where the descriptor closes `retry` to its five declared keys'], +]); + +/** + * [#21982] Does the spec judge KEY MEMBERSHIP on this node type's `config`, at + * every door that parses a flow — `FlowSchema.parse`, `objectstack validate`, + * `objectstack compile`, the metadata save door and `registerFlow` (which + * parses first)? True for every builtin in {@link + * getBuiltinNodeConfigContracts} but the ones in + * {@link BUILTIN_KEYS_JUDGED_AT_REGISTRATION}: each of those contracts is + * strict, and its executor parses it before anything else. + * + * The ONE judge of a builtin's undeclared key. `registerFlow`'s descriptor walk + * (`service-automation` `validateNodeConfigKeys`) asks this and stands aside + * for every type it answers `true` for, so no type has two judges; the walk + * keeps every other type — `try_catch`, and the plugin node types the spec + * does not declare. Measured before the move: on every type it answers `true` + * for, the descriptor's declared key sets, at every position the walk descends + * to, equal the keys the contract accepts there, so the move changed no verdict + * at registration and only added the build doors. + * + * `false` for a declared PLUGIN contract (`approval`): that one is judged whole + * by {@link flowNodeConfigRefusals}, a different arm, and is not a builtin. + */ +export function builtinNodeConfigKeysJudged(nodeType: string): boolean { + return getBuiltinNodeConfigContracts().has(nodeType) && !BUILTIN_KEYS_JUDGED_AT_REGISTRATION.has(nodeType); +} + +/** + * [#21982] Does an `unrecognized_keys` issue sit INSIDE an ADR-0031 region + * rather than on the region object itself? The issue's `path` is the object + * that holds the key: `[]` for the config, `['body']` / `['try']` for a region + * object (arity `one`), `['branches', 0]` for one of a `many` slot's regions. + * A key on the region object is the container's, judged here like any other + * config key; anything deeper — a region node's or edge's own key — is the + * region's, refused at `registerFlow` by `validateControlFlow` and never + * re-reported against the container (the region policy `flow.zod.ts` states). + */ +function unknownKeyInsideRegion(nodeType: string, path: ReadonlyArray): boolean { + if (path.length === 0) return false; + const slot = (FLOW_REGION_SLOTS_BY_TYPE.get(nodeType) ?? []).find((s) => s.key === path[0]); + if (!slot) return false; + return path.length > (slot.arity === 'many' ? 2 : 1); } /** @@ -451,28 +491,50 @@ function nodeConfigKeyMissingMessage(nodeType: string, key: string): string { ); } +/** + * How a {@link nodeConfigRefusedByContractMessage} refusal closes: `value` + * prescribes a value the contract accepts (a plain value finding); `key` + * (#21982) prescribes renaming or removing an undeclared key on a builtin, + * whose contract sentence says what is wrong but, for a key with no + * did-you-mean and no guidance, not what to do; `none` adds nothing (a rule's + * text, and a whole-judged plugin contract's, already say what to write). + */ +type RefusalClosing = 'value' | 'key' | 'none'; + /** * [#21850] The refusal for a key a WHOLE-judged contract does not declare, or a * value it refuses — and (#21898) a value a builtin contract refuses, and * (#21982) a key a key-judged builtin contract does not declare — where the - * author wrote it: the contract's own sentence, - * inside one that names the node type and the key. `prescribe` adds the closing - * instruction for a plain value finding; an unknown key's text (with its - * did-you-mean) and a rule's text already say what to write. + * author wrote it: the contract's own sentence, inside one that names the node + * type and the key, closed per {@link RefusalClosing}. + * + * `parsedAtRun` false is a key on a config its executor does not parse — a + * body-less legacy `loop` (#21982): the run would not fail there, but nothing + * reads the key, and `registerFlow` refused the flow on it before the build + * doors did, so the sentence says that instead. */ function nodeConfigRefusedByContractMessage( nodeType: string, key: string, contractMessage: string, - prescribe: boolean, + closing: RefusalClosing, + parsedAtRun = true, ): string { const reason = contractMessage.trim(); return ( `This \`${nodeType}\` node's config is refused at \`${key}\` by the ${nodeType} contract: ` - + `${/[.!?]$/.test(reason) ? reason : `${reason}.`} Its executor parses the config against that contract ` - + 'before it does anything else and refuses the node on any finding, so every run that reached this node ' - + 'would fail there; the config is metadata, and re-running changes nothing.' - + (prescribe ? ` Write a value the ${nodeType} contract accepts at \`${key}\`.` : '') + + `${/[.!?]$/.test(reason) ? reason : `${reason}.`} ` + + (parsedAtRun + ? 'Its executor parses the config against that contract before it does anything else and refuses the ' + + 'node on any finding, so every run that reached this node would fail there; the config is metadata, ' + + 'and re-running changes nothing.' + : 'Its executor reads no key the contract does not declare, and registration refuses the whole flow on ' + + 'one, so the flow would never run; the config is metadata, and re-running changes nothing.') + + (closing === 'value' ? ` Write a value the ${nodeType} contract accepts at \`${key}\`.` : '') + + (closing === 'key' + ? ` Rename the key to one the ${nodeType} contract declares there, or remove it: an undeclared key is never ` + + 'read, so it can only be a typo or dead config.' + : '') ); } @@ -518,17 +580,20 @@ function unrecognizedKeysOf(issue: { readonly code: string }): readonly string[] * next one's, a tombstoned key nobody's here), nor a region slot, a * `predicate` / `value` ledger slot, a run-resolved key, or any value * carrying a `{token}`: never refused for its pre-interpolation type. - * - (#21982) A key the contract does not declare, on a builtin whose - * descriptor publishes no `configSchema` ({@link builtinKeysJudged}: - * `script`, `subflow`) → `node-config-refused-by-contract`, anchored at the - * key, one refusal per undeclared key (`bogusKey` on a `script`), in the + * - (#21982) A key the contract does not declare, on every builtin + * {@link builtinNodeConfigKeysJudged} judges (all but `try_catch`) → + * `node-config-refused-by-contract`, anchored at the key, one refusal per + * undeclared key (`bogusKey` on a `notify`; `fields[0].visibleIf` on a + * `screen` field; `body.bogusKey` on a `loop`'s region object), in the * contract's own words — its prescription for a known slip included - * (`subflow` `timeoutMs` belongs on the node). Registration's undeclared-key - * walk reads the descriptor schema these types do not publish, and their - * executors parse the strict contract before anything else, so this is the - * one door before the run that refuses them. Every other builtin's - * undeclared key stays registration's, and a tombstoned key (a - * `retiredKey()`) keeps the path it had. + * (`fieldValues` → `fields`, `bulk` → `multi`, `visibleIf` → + * `visibleWhen`, a `subflow` `timeoutMs` that belongs on the node) — and + * closed with the rename-or-remove remedy. Each executor parses its strict + * contract before anything else, so the run refuses such a node; this arm + * is the one judge of it at every door, and `registerFlow`'s descriptor + * walk stands aside for these types. A key INSIDE a region — a region + * node's or edge's own key — is the region's (`validateControlFlow`), and + * a tombstoned key (a `retiredKey()`) keeps the path it had. * * Where the build cannot read the config whole, it reads only what is sound, * and each such type is named here, not skipped in silence: @@ -538,13 +603,16 @@ function unrecognizedKeysOf(issue: { readonly code: string }): readonly string[] * the identity), a rule only when the whole config carries none, and * `signingSecret` never (the credential channel may supply it); * - `loop` parses only with a `body` (`parsedWhen`), so a legacy flat-graph - * loop is judged for nothing, and a loop with a body on its own keys - * (`collection`, `iteratorVariable`, `indexVariable`, `maxIterations`); + * loop is judged on key membership alone (#21982: nothing reads an + * undeclared key on either path, and registration refused one there + * already), and a loop with a body on its own keys as well (`collection`, + * `iteratorVariable`, `indexVariable`, `maxIterations`); * - the region containers — `loop` (`body`), `parallel` (`branches`) and * `try_catch` (`try`, `catch`) — hold regions, judged as graphs of their * own; a container is judged on the keys beside them (`try_catch`'s * `errorVariable` and `retry`), so `parallel`, whose one key is its region - * slot, is judged for presence alone. + * slot, is judged for presence and for the keys on each region object + * alone. * * A key only the conversion layer spells canonically (`object` → * `objectName`, `flow` → `flowName`, …) is judged AFTER the conversion at @@ -621,12 +689,17 @@ export function flowNodeConfigRefusals(nodeType: string, config: unknown): FlowN const contract = builtin ?? declared; if (!contract) return out; const whole = declared !== undefined; - // [#21982] Key membership: a declared plugin contract's always, a builtin's - // only where no descriptor `configSchema` lets registration judge it. - const keysJudged = whole || builtinKeysJudged(nodeType); + // [#21982] Key membership: a declared plugin contract's always, and a + // builtin's wherever the spec is its one judge (every builtin but the ones + // whose key set stays registration's). + const keysJudged = whole || builtinNodeConfigKeysJudged(nodeType); const authored = config ?? {}; if (!isRecord(authored)) return out; - if (contract.parsedWhen && !contract.parsedWhen(authored)) return out; + // [#21982] `parsedWhen` gates presence and values, never key membership: a + // body-less `loop` is judged on its keys alone, which is what registration + // refused there before the build doors judged keys. + const parsedAtRun = !contract.parsedWhen || contract.parsedWhen(authored); + if (!parsedAtRun && !keysJudged) return out; const result = contract.schema.safeParse(authored); if (result.success) return out; const seen = new Set(); @@ -635,20 +708,24 @@ export function flowNodeConfigRefusals(nodeType: string, config: unknown): FlowN // An unknown key's issue sits on the object that holds it (the config // itself for a top-level key, so its `path` is empty): anchor one // refusal at each key the author wrote, with the contract's sentence. - for (const unknownKey of unrecognizedKeysOf(issue)) { + // A key inside a region (a region node's or edge's own key) is the + // region's, never the container's. + const keys = unknownKeyInsideRegion(nodeType, issue.path) ? [] : unrecognizedKeysOf(issue); + for (const unknownKey of keys) { const key = ledgerPathOf([...issue.path, unknownKey]); if (seen.has(key)) continue; seen.add(key); out.push({ code: 'node-config-refused-by-contract', params: { nodeType, key }, - message: nodeConfigRefusedByContractMessage(nodeType, key, issue.message, false), + message: nodeConfigRefusedByContractMessage(nodeType, key, issue.message, whole ? 'none' : 'key', parsedAtRun), source: '', path: key, }); } if (issue.code === 'unrecognized_keys') continue; } + if (!parsedAtRun) continue; if (issue.path.length === 0) continue; if (insideRegion(nodeType, issue.path)) continue; if (insideValueSlot(nodeType, issue.path)) continue; @@ -663,7 +740,7 @@ export function flowNodeConfigRefusals(nodeType: string, config: unknown): FlowN out.push({ code: 'node-config-refused-by-contract', params: { nodeType, key }, - message: nodeConfigRefusedByContractMessage(nodeType, key, issue.message, issue.code !== 'custom'), + message: nodeConfigRefusedByContractMessage(nodeType, key, issue.message, issue.code !== 'custom' ? 'value' : 'none'), source: '', path: key, }); diff --git a/packages/spec/src/automation/flow.zod.ts b/packages/spec/src/automation/flow.zod.ts index 8d7bee83d29..701edc0f5c5 100644 --- a/packages/spec/src/automation/flow.zod.ts +++ b/packages/spec/src/automation/flow.zod.ts @@ -169,14 +169,15 @@ export const FLOW_PAUSE_CAPABLE_NODE_TYPES: readonly string[] = [ * (#20316): a key the node's executor contract requires, left out, and a * `decision` branch list the executor cannot read. A builtin VALUE rule with * it (#21898): a value a builtin node's executor contract refuses, where the - * build can know what the run will parse. And KEY MEMBERSHIP only where the - * build is the one door before the run: the declared `approval` contract is - * judged whole (#21850), and a key a `script` or `subflow` executor contract - * does not declare is refused (#21982) — those two descriptors publish no - * `configSchema` for registration's undeclared-key walk to read. Every other - * builtin's undeclared key is judged at `registerFlow` against its descriptor, - * and so is a plugin type's whose contract the spec does not declare; the - * flow parse does not judge those keys. + * build can know what the run will parse. And KEY MEMBERSHIP: the declared + * `approval` contract is judged whole (#21850), and a key a builtin's executor + * contract does not declare is refused (#21982) for every builtin + * `builtinNodeConfigKeysJudged` names — all but `try_catch` — so this parse is + * the one judge of those keys at every door, and `registerFlow`'s descriptor + * walk stands aside for them. `try_catch`'s undeclared key is judged at + * `registerFlow` against its descriptor (its contract's `retry` strips an + * unknown key where the descriptor closes it), and so is a plugin type's whose + * contract the spec does not declare; the flow parse does not judge those keys. */ /** @@ -1487,19 +1488,21 @@ export const FlowSchema = lazySchema(() => strictObject( // refuses is refused too (`create_record` `outputVariable: 42`, a screen // field `min: '1'`), where the build can know what the run parses — never // a value carrying a `{token}`, a region slot, a ledger predicate or - // value slot, or `http`'s run-resolved `signingSecret`; an undeclared key - // stays where it is judged today. The one plugin node contract the spec - // declares, `approval` (#21850), is judged WHOLE — its executor refuses - // the node on any contract finding — so its undeclared keys are refused - // here too; + // value slot, or `http`'s run-resolved `signingSecret`. A KEY its + // contract does not declare is refused too (#21982), for every builtin + // `builtinNodeConfigKeysJudged` names (all but `try_catch`, whose keys + // `registerFlow`'s descriptor walk keeps), a body-less legacy `loop` + // included. The one plugin node contract the spec declares, `approval` + // (#21850), is judged WHOLE — its executor refuses the node on any + // contract finding — so its undeclared keys are refused here too; // - 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 and VALUE rule for a builtin, never a key-set closure: the node - // `config` stays the open record the header of this module describes, and - // only the approval node's declared contract closes its key set. Walked with + // The node `config` stays the open record the header of this module + // describes: the executor contracts close its key set through this one + // judge, never a re-declared shape here. 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. diff --git a/packages/spec/src/migrations/entries/semantic/18.flow-builtin-node-config-undeclared-keys-refused.ts b/packages/spec/src/migrations/entries/semantic/18.flow-builtin-node-config-undeclared-keys-refused.ts new file mode 100644 index 00000000000..c7aa1ffb179 --- /dev/null +++ b/packages/spec/src/migrations/entries/semantic/18.flow-builtin-node-config-undeclared-keys-refused.ts @@ -0,0 +1,83 @@ +// Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license. + +import type { SemanticMigration } from '../../types.js'; + +// #21982 — the D3 entry for the build doors refusing an UNDECLARED KEY on the +// builtin node types whose undeclared keys only `registerFlow` judged until +// now: `get_record`, `create_record`, `update_record`, `delete_record`, +// `notify`, `http`, `screen`, `map`, `loop` and `parallel`. It completes the +// key half of the executor-contract arm of `flowNodeConfigRefusals` that +// `flow-script-subflow-config-undeclared-keys-refused` opened for `script` and +// `subflow`, and makes that arm the one judge of a builtin's undeclared key: +// `registerFlow`'s descriptor walk stands aside for these types. It narrows the +// build doors' accept set to what registration already refused; no key is +// removed, so there is no tombstone and no RETIRED_KEYS_BY_MAJOR row, and no D2 +// conversion exists: the platform cannot know what an undeclared key was meant +// to be. +// +// No backticks and no pipes in `surface` — build-upgrade-guide.ts renders it +// inside a code span and a table cell. +export const entry: SemanticMigration = { + id: 'flow-builtin-node-config-undeclared-keys-refused', + surface: + 'a get_record, create_record, update_record, delete_record, notify, http, screen, map, loop or parallel ' + + 'flow node whose config carries a key its executor contract does not declare — a typo (titl), a key ' + + 'the walk at registration already named (fieldValues on a write node, bulk on update_record, visibleIf ' + + 'on a screen field), a key copied from another node type (outputVariable on an http node, flowName on a ' + + 'loop), or a key nothing reads (bogusKey) — at the config itself, on a screen field or one of its ' + + 'options, on a loop body region object or on a parallel branch, a body-less legacy loop included. ' + + 'Never a key inside a free-form map (a filter, fields, headers, defaults, input, payload or templateData ' + + 'key is author data), never a key on a region node or edge (the region check owns those), and never a ' + + 'try_catch key, which registration keeps judging against its descriptor. Reachable wherever a flow is ' + + 'authored or stored: defineStack({ flows }) sources, defineFlow(), an exported stack passed to ' + + 'objectstack validate or objectstack compile, a flow saved from the Studio flow designer, and a flow row ' + + 'already sitting in sys_metadata', + replacement: + 'the key the contract declares, or no key: rename a typo to the declared key it meant (the refusal ' + + 'carries the contract\'s did-you-mean for a near miss), follow the contract\'s own prescription for a ' + + 'known slip (`fieldValues` → `fields`, `bulk` / `all` / `multiple` → `multi: true`, `options: { multi }` → ' + + 'a top-level `multi`, a screen field\'s `visibleIf` → `visibleWhen`, a loop\'s `itemVariable` → ' + + '`iteratorVariable`), and delete a key nothing reads (an `http` node\'s `outputVariable` among them: the ' + + 'http executor binds no output variable)', + reason: + 'Each of these executors (`service-automation` `builtin/crud-nodes.ts`, `notify-node.ts`, ' + + '`http-nodes.ts`, `screen-nodes.ts`, `map-node.ts`, `loop-node.ts`, `parallel-node.ts`) parses the ' + + 'node\'s `config` against a strict contract before it acts. Until now the build doors\' executor-contract ' + + 'arm held key membership back on these types, on the premise that registration judges it: ' + + '`registerFlow`\'s undeclared-key walk (`validateNodeConfigKeys`) refuses such a key against the node ' + + 'type descriptor\'s `configSchema`. So a `notify` node carrying `bogusKey` passed `FlowSchema.parse`, ' + + '`objectstack validate` and `objectstack compile` (which copied it into the artifact), and then ' + + 'registration refused the whole flow: at boot it was skipped with a warn, and a flow saved from Studio ' + + 'was stored and then silently not registered. The one judge `FlowSchema.parse`, ' + + '`AutomationEngine.registerFlow` (which parses first), `objectstack validate` and the metadata save door ' + + 'share (`flowNodeConfigRefusals`) now refuses such a key on these types as `node-config-refused-by-contract`, ' + + 'anchored at the key, one refusal per key, in the contract\'s own words and closed with the ' + + 'rename-or-remove remedy, and the descriptor walk stands aside for every type that judge covers ' + + '(`builtinNodeConfigKeysJudged`), so each type has one judge. Measured before the move: on each of ' + + 'these types the descriptor\'s declared key sets, at every position the walk descends to, equal the ' + + 'keys the contract accepts there, so registration refuses exactly what it refused before. ⚠️ ' + + '`try_catch` is the one builtin not moved: its contract\'s `retry` is the shared `RetryPolicySchema`, ' + + 'which strips an unknown key, while its descriptor closes `retry` to five keys, so its undeclared keys ' + + 'stay registration\'s. ⚠️ A body-less legacy `loop` is not parsed at run time, and it is judged here on ' + + 'key membership alone, which is what registration refused there already. ⚠️ A spelling an ADR-0087 D2 ' + + 'conversion still rewrites at load (`object` and `filters` on a CRUD node, `to` / `subject` / `body` / ' + + '`url` on a `notify`, `flow` on a `map`) is converted before the judge at every door that converts ' + + 'first; met by a direct `FlowSchema.parse` or `defineFlow()` it is refused like any other undeclared ' + + 'key. ⚠️ No D2 conversion: the platform cannot know what an undeclared key was meant to be. ⚠️ Where ' + + 'such a node already sits the whole flow is refused, as registration already refused it: 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; a save from ' + + 'Studio answers 422 naming the key. 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.bogusKey`, `nodes.N.config.fields.0.visibleIf`, or the region ' + + 'path `nodes.N.config.body.nodes.M.config…`), `objectstack validate` prints the same path, and ' + + '`validateStackExpressions` phrases it as `node \'n\' (notify) config.bogusKey`. For each hit rename or ' + + 'delete the key 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 — that warn line is the locator for a row that exists only in ' + + '`sys_metadata`. A node of these types whose keys its contract declares parses and registers ' + + 'byte-identically to before.', +}; diff --git a/packages/spec/src/migrations/registry.ts b/packages/spec/src/migrations/registry.ts index 989c71ffef8..493f0d01c8d 100644 --- a/packages/spec/src/migrations/registry.ts +++ b/packages/spec/src/migrations/registry.ts @@ -5648,6 +5648,22 @@ const STEP18_RATIONALE: readonly RationaleFragment[] = [ + 'the platform cannot know what the author meant. Its D3 record is the semantic entry ' + '`flow-approval-node-config-contract-refused`.', }, + { + id: 'flow-builtin-node-config-undeclared-keys-refused', + order: 89, + text: + 'Then the builtin key arm covers every builtin whose contract registration could judge: a key the ' + + 'executor contract of a `get_record`, `create_record`, `update_record`, `delete_record`, `notify`, ' + + '`http`, `screen`, `map`, `loop` or `parallel` node does not declare is refused at parse, at ' + + '`nodes.N.config.`, with the same `node-config-refused-by-contract` code, closed with the ' + + 'rename-or-remove remedy. Registration\'s descriptor walk refused those keys already, after ' + + '`objectstack validate` and `objectstack compile` had passed them, and it now stands aside for those ' + + 'types, so each has one judge; the declared key sets were measured equal first, so registration ' + + 'refuses what it refused before. `try_catch` stays registration\'s: its contract\'s `retry` strips an ' + + 'unknown key where its descriptor closes it. No key is removed, so there is no tombstone, and no D2 ' + + 'conversion exists. Its D3 record is the semantic entry ' + + '`flow-builtin-node-config-undeclared-keys-refused`.', + }, { id: 'flow-builtin-node-config-values-refused', order: 85, @@ -13436,6 +13452,85 @@ const step18: MigrationStep = { + 'row that exists only in `sys_metadata`. An approval node the contract accepts parses and ' + 'registers byte-identically to before.', }, + // #21982 — the D3 entry for the build doors refusing an UNDECLARED KEY on the + // builtin node types whose undeclared keys only `registerFlow` judged until + // now: `get_record`, `create_record`, `update_record`, `delete_record`, + // `notify`, `http`, `screen`, `map`, `loop` and `parallel`. It completes the + // key half of the executor-contract arm of `flowNodeConfigRefusals` that + // `flow-script-subflow-config-undeclared-keys-refused` opened for `script` and + // `subflow`, and makes that arm the one judge of a builtin's undeclared key: + // `registerFlow`'s descriptor walk stands aside for these types. It narrows the + // build doors' accept set to what registration already refused; no key is + // removed, so there is no tombstone and no RETIRED_KEYS_BY_MAJOR row, and no D2 + // conversion exists: the platform cannot know what an undeclared key was meant + // to be. + // + // No backticks and no pipes in `surface` — build-upgrade-guide.ts renders it + // inside a code span and a table cell. + { + id: 'flow-builtin-node-config-undeclared-keys-refused', + surface: + 'a get_record, create_record, update_record, delete_record, notify, http, screen, map, loop or parallel ' + + 'flow node whose config carries a key its executor contract does not declare — a typo (titl), a key ' + + 'the walk at registration already named (fieldValues on a write node, bulk on update_record, visibleIf ' + + 'on a screen field), a key copied from another node type (outputVariable on an http node, flowName on a ' + + 'loop), or a key nothing reads (bogusKey) — at the config itself, on a screen field or one of its ' + + 'options, on a loop body region object or on a parallel branch, a body-less legacy loop included. ' + + 'Never a key inside a free-form map (a filter, fields, headers, defaults, input, payload or templateData ' + + 'key is author data), never a key on a region node or edge (the region check owns those), and never a ' + + 'try_catch key, which registration keeps judging against its descriptor. Reachable wherever a flow is ' + + 'authored or stored: defineStack({ flows }) sources, defineFlow(), an exported stack passed to ' + + 'objectstack validate or objectstack compile, a flow saved from the Studio flow designer, and a flow row ' + + 'already sitting in sys_metadata', + replacement: + 'the key the contract declares, or no key: rename a typo to the declared key it meant (the refusal ' + + 'carries the contract\'s did-you-mean for a near miss), follow the contract\'s own prescription for a ' + + 'known slip (`fieldValues` → `fields`, `bulk` / `all` / `multiple` → `multi: true`, `options: { multi }` → ' + + 'a top-level `multi`, a screen field\'s `visibleIf` → `visibleWhen`, a loop\'s `itemVariable` → ' + + '`iteratorVariable`), and delete a key nothing reads (an `http` node\'s `outputVariable` among them: the ' + + 'http executor binds no output variable)', + reason: + 'Each of these executors (`service-automation` `builtin/crud-nodes.ts`, `notify-node.ts`, ' + + '`http-nodes.ts`, `screen-nodes.ts`, `map-node.ts`, `loop-node.ts`, `parallel-node.ts`) parses the ' + + 'node\'s `config` against a strict contract before it acts. Until now the build doors\' executor-contract ' + + 'arm held key membership back on these types, on the premise that registration judges it: ' + + '`registerFlow`\'s undeclared-key walk (`validateNodeConfigKeys`) refuses such a key against the node ' + + 'type descriptor\'s `configSchema`. So a `notify` node carrying `bogusKey` passed `FlowSchema.parse`, ' + + '`objectstack validate` and `objectstack compile` (which copied it into the artifact), and then ' + + 'registration refused the whole flow: at boot it was skipped with a warn, and a flow saved from Studio ' + + 'was stored and then silently not registered. The one judge `FlowSchema.parse`, ' + + '`AutomationEngine.registerFlow` (which parses first), `objectstack validate` and the metadata save door ' + + 'share (`flowNodeConfigRefusals`) now refuses such a key on these types as `node-config-refused-by-contract`, ' + + 'anchored at the key, one refusal per key, in the contract\'s own words and closed with the ' + + 'rename-or-remove remedy, and the descriptor walk stands aside for every type that judge covers ' + + '(`builtinNodeConfigKeysJudged`), so each type has one judge. Measured before the move: on each of ' + + 'these types the descriptor\'s declared key sets, at every position the walk descends to, equal the ' + + 'keys the contract accepts there, so registration refuses exactly what it refused before. ⚠️ ' + + '`try_catch` is the one builtin not moved: its contract\'s `retry` is the shared `RetryPolicySchema`, ' + + 'which strips an unknown key, while its descriptor closes `retry` to five keys, so its undeclared keys ' + + 'stay registration\'s. ⚠️ A body-less legacy `loop` is not parsed at run time, and it is judged here on ' + + 'key membership alone, which is what registration refused there already. ⚠️ A spelling an ADR-0087 D2 ' + + 'conversion still rewrites at load (`object` and `filters` on a CRUD node, `to` / `subject` / `body` / ' + + '`url` on a `notify`, `flow` on a `map`) is converted before the judge at every door that converts ' + + 'first; met by a direct `FlowSchema.parse` or `defineFlow()` it is refused like any other undeclared ' + + 'key. ⚠️ No D2 conversion: the platform cannot know what an undeclared key was meant to be. ⚠️ Where ' + + 'such a node already sits the whole flow is refused, as registration already refused it: 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; a save from ' + + 'Studio answers 422 naming the key. 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.bogusKey`, `nodes.N.config.fields.0.visibleIf`, or the region ' + + 'path `nodes.N.config.body.nodes.M.config…`), `objectstack validate` prints the same path, and ' + + '`validateStackExpressions` phrases it as `node \'n\' (notify) config.bogusKey`. For each hit rename or ' + + 'delete the key 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 — that warn line is the locator for a row that exists only in ' + + '`sys_metadata`. A node of these types whose keys its contract declares parses and registers ' + + 'byte-identically to before.', + }, // #21898 — the D3 entry for the build doors refusing a VALUE a builtin flow // node's executor contract refuses: the value half of the executor-contract arm // of `flowNodeConfigRefusals` (`flow-node-config-refusals.ts`), beside the From 4cd522f6a23a61ed976ece6a265480f0f2db2148 Mon Sep 17 00:00:00 2001 From: Claude Date: Thu, 8 Oct 2026 13:11:35 +0000 Subject: [PATCH 02/10] test(spec): re-point the builtin key pins to the spec arm (wip) Claude-Session: https://claude.ai/code/session_01DhTqaEHqPVSVnAkjG3jywn Co-authored-by: Claude --- ...flow-approval-node-config-contract.test.ts | 11 +- .../flow-builtin-node-config-keys.test.ts | 198 +++++++++++++++--- ...-decision-branch-expression-absent.test.ts | 2 +- .../automation/flow-node-config-refusals.ts | 43 ++-- .../flow-predicate-slot-blank.test.ts | 2 +- ...tin-node-config-undeclared-keys-refused.ts | 9 +- packages/spec/src/migrations/registry.ts | 9 +- 7 files changed, 215 insertions(+), 59 deletions(-) diff --git a/packages/spec/src/automation/flow-approval-node-config-contract.test.ts b/packages/spec/src/automation/flow-approval-node-config-contract.test.ts index 7f18e75d0c3..8157598b17f 100644 --- a/packages/spec/src/automation/flow-approval-node-config-contract.test.ts +++ b/packages/spec/src/automation/flow-approval-node-config-contract.test.ts @@ -175,17 +175,22 @@ describe('what stays accepted (lit controls)', () => { expect(issuesOf(flowWith({ approvers: APPROVERS }))).toEqual([]); }); - it('CONTROL: the builtin arm judges no key membership — an undeclared key on a builtin node still parses', () => { + it('CONTROL: the approval contract judges no builtin node — a builtin\'s undeclared key is the builtin key arm\'s', () => { + // [#21982] A builtin's undeclared key is refused now too, but by the builtin + // arm, in the builtin contract's words — never by the approval contract. + const config = { url: 'https://example.test', bogusKey: 1 }; const flow = { ...flowWith(VALID), nodes: [ { id: 'start', type: 'start', label: 'Start' }, - { id: 'call', type: 'http', label: 'Call', config: { url: 'https://example.test', bogusKey: 1 } }, + { id: 'call', type: 'http', label: 'Call', config }, { id: 'done', type: 'end', label: 'Done' }, ], edges: [{ id: 'e1', source: 'start', target: 'call' }, { id: 'e2', source: 'call', target: 'done' }], }; - expect(issuesOf(flow)).toEqual([]); + const [builtinRefusal] = flowNodeConfigRefusals('http', config); + expect(builtinRefusal!.params).toEqual({ nodeType: 'http', key: 'bogusKey' }); + expect(issuesOf(flow)).toEqual([{ code: 'custom', path: 'nodes.1.config.bogusKey', message: builtinRefusal!.message }]); // …and the builtin map, the executor-reconciled one, did not gain the plugin type. expect(getBuiltinNodeConfigContracts().has('approval')).toBe(false); }); diff --git a/packages/spec/src/automation/flow-builtin-node-config-keys.test.ts b/packages/spec/src/automation/flow-builtin-node-config-keys.test.ts index 74c6f18cda5..a6362697a36 100644 --- a/packages/spec/src/automation/flow-builtin-node-config-keys.test.ts +++ b/packages/spec/src/automation/flow-builtin-node-config-keys.test.ts @@ -2,22 +2,26 @@ /** * [#21982] The build doors refuse a KEY a builtin node's executor contract does - * not declare, where no other door before the run judges it — the key half of - * the executor-contract arm of `flowNodeConfigRefusals`, beside the value half - * (#21898) and the presence half (#20316). + * not declare — the key half of the executor-contract arm of + * `flowNodeConfigRefusals`, beside the value half (#21898) and the presence + * half (#20316) — and that arm is the ONE judge of a builtin's undeclared key. * - * `script` and `subflow` publish no descriptor `configSchema`, so - * `registerFlow`'s undeclared-key walk skips them, while their executors parse - * the strict contract and refuse the node on an undeclared key at every run. A - * `script` node carrying `bogusKey` passed `FlowSchema`, `objectstack validate`, - * `objectstack compile` and `registerFlow`, and then failed every run. The pins - * below hold the refusal at every door that parses a flow; the controls hold - * what stays as it was: + * Pass 1 covered `script` and `subflow`, which publish no descriptor + * `configSchema`, so no door before the run judged their keys. The remainder + * covers every other builtin but `try_catch`: a `notify` node carrying + * `bogusKey` passed `FlowSchema`, `objectstack validate` and `objectstack + * compile`, and `registerFlow`'s descriptor walk then refused the whole flow at + * boot. The spec now refuses it at every door, and the walk stands aside for + * every type `builtinNodeConfigKeysJudged` names (pinned in + * `service-automation` `builtin/config-unknown-keys.test.ts`). The controls + * hold what stays as it was: * - * - a valid `script` / `subflow` node, and the measured node without its extra key; - * - a builtin WITH a descriptor `configSchema`: its undeclared key stays - * registration's, judged against the descriptor; - * - `decision`, schemaless but with no executor contract; + * - a valid node of each type, and the measured node without its extra key; + * - `try_catch`, whose contract's `retry` strips an unknown key where its + * descriptor closes it: its keys stay registration's; + * - a key at or under a region slot, which is `validateControlFlow`'s; + * - a key inside a free-form map (author data); + * - `decision`, schemaless with no executor contract; * - a tombstoned retired `script` key, which keeps the path it had. */ @@ -26,17 +30,35 @@ import { describe, expect, it } from 'vitest'; import { getMetadataTypeSchema } from '../kernel/metadata-type-schemas'; import { MIGRATIONS_BY_MAJOR } from '../migrations/registry'; import { ArtifactStagePackageBodySchema, ObjectStackDefinitionSchema, defineStack } from '../stack.zod'; -import { flowNodeConfigRefusals, getBuiltinNodeConfigContracts } from './flow-node-config-refusals'; +import { builtinNodeConfigKeysJudged, flowNodeConfigRefusals, getBuiltinNodeConfigContracts } from './flow-node-config-refusals'; import { FlowSchema } from './flow.zod'; -import { SCHEMALESS_NODE_CONFIG_SCHEMAS } from './schemaless-node-config.zod'; const ENTRY_ID = 'flow-script-subflow-config-undeclared-keys-refused'; +const REMAINDER_ENTRY_ID = 'flow-builtin-node-config-undeclared-keys-refused'; type Config = Record; /** The measured node: `summarize` in the showcase's `showcase_task_completed`. */ const MEASURED: Config = { function: 'summarizeCompletedTask', inputs: { taskId: '{record.id}' }, outputVariable: 'summary' }; const SUBFLOW: Config = { flowName: 'child_flow', input: { id: '{record.id}' }, outputVariable: 'out' }; +/** The remainder's measured node: a `notify` with `bogusKey` (#21982's pass-1 report). */ +const NOTIFY: Config = { recipients: '{record.owner}', title: 'Done', message: '{summary}' }; + +/** A valid config of every builtin the key arm judges — the controls each type is probed against. */ +const VALID: Readonly> = { + get_record: { objectName: 'task', filter: { status: 'open' }, limit: 5, outputVariable: 'rows' }, + create_record: { objectName: 'task', fields: { title: 'X' }, outputVariable: 'created' }, + update_record: { objectName: 'task', filter: { id: '{recordId}' }, fields: { title: 'X' } }, + delete_record: { objectName: 'task', filter: { id: '{recordId}' } }, + notify: NOTIFY, + http: { url: 'https://example.test/hook', method: 'POST', headers: { 'X-Any': 'y' } }, + screen: { title: 'T', fields: [{ name: 'a', type: 'select', options: [{ label: 'A', value: 'a' }] }] }, + script: MEASURED, + subflow: SUBFLOW, + map: { collection: '{rows}', flowName: 'child_flow', input: { id: '{item.id}' } }, + loop: { collection: '{rows}', body: { nodes: [], edges: [] } }, + parallel: { branches: [{ name: 'a', nodes: [], edges: [] }, { name: 'b', nodes: [], edges: [] }] }, +}; /** start → one node of `type` → end. */ function flowWith(type: string, config: unknown, name = 'key_probe') { @@ -120,6 +142,78 @@ describe('the measured node: refused at save, with its location', () => { }); }); +describe('the remainder: every other builtin but try_catch, refused at save', () => { + it('a notify node with bogusKey is refused at nodes.1.config.bogusKey, with the rename-or-remove remedy', () => { + const config = { ...NOTIFY, bogusKey: 1 }; + const issues = issuesOf(flowWith('notify', config)); + expect(issues.map(({ code, path }) => ({ code, path }))).toEqual([{ code: 'custom', path: 'nodes.1.config.bogusKey' }]); + const [refusal] = flowNodeConfigRefusals('notify', config); + expect({ code: refusal!.code, params: refusal!.params, path: refusal!.path }).toEqual({ + code: 'node-config-refused-by-contract', params: { nodeType: 'notify', key: 'bogusKey' }, path: 'bogusKey', + }); + expect(issues[0]!.message).toBe(refusal!.message); + expect(refusal!.message).toContain(contractSentence('notify', config)); + // The remedy the descriptor walk's rejection carried, kept: rename or remove. + expect(refusal!.message).toMatch(/Rename the key .* or remove it/); + }); + + it('CONTROL: the measured notify node without its extra key parses clean', () => { + expect(issuesOf(flowWith('notify', NOTIFY))).toEqual([]); + }); + + it('every judged type refuses a top-level bogusKey on an otherwise valid config, anchored at the key', () => { + for (const [type, config] of Object.entries(VALID)) { + expect(flowNodeConfigRefusals(type, { ...config, bogusKey: 1 }).map(({ code, path }) => ({ code, path })), type) + .toEqual([{ code: 'node-config-refused-by-contract', path: 'bogusKey' }]); + } + }); + + it('the walk\'s per-key prescriptions live on in the contracts\' own sentences', () => { + for (const [type, config, path, prescription] of [ + ['create_record', { objectName: 'task', fieldValues: { a: 1 } }, 'fieldValues', /The write map is `fields`/], + ['update_record', { objectName: 'task', filter: { id: 'x' }, bulk: true }, 'bulk', /`multi: true`/], + ['delete_record', { objectName: 'task', filter: { id: 'x' }, options: { multi: true } }, 'options', /`multi: true`/], + ['screen', { fields: [{ name: 'qty', visibleIf: 'x' }] }, 'fields[0].visibleIf', /`visibleWhen`/], + ] as const) { + const refusals = flowNodeConfigRefusals(type, config); + expect(refusals.map((r) => r.path), type).toEqual([path]); + expect(refusals[0]!.message, type).toMatch(prescription); + } + }); + + it('a key on a screen field or one of its options is refused where the author wrote it', () => { + const config = { fields: [{ name: 'a', type: 'select', options: [{ label: 'A', value: 'a', bogusKey: 1 }], other: 2 }] }; + expect(flowNodeConfigRefusals('screen', config).map((r) => r.path).sort()) + .toEqual(['fields[0].options[0].bogusKey', 'fields[0].other']); + expect(issuesOf(flowWith('screen', config)).map((i) => i.path).sort()) + .toEqual(['nodes.1.config.fields.0.options.0.bogusKey', 'nodes.1.config.fields.0.other']); + }); + + it('a body-less legacy loop is judged on key membership alone, as registration judged it', () => { + // Keys: refused, whatever `parsedWhen` says. + const [refusal] = flowNodeConfigRefusals('loop', { collection: 'rows', bogusKey: 1 }); + expect({ code: refusal!.code, path: refusal!.path }).toEqual({ code: 'node-config-refused-by-contract', path: 'bogusKey' }); + // The run does not parse this form, so the sentence names registration, not a failing run. + expect(refusal!.message).toContain('registration refuses the whole flow'); + expect(issuesOf(flowWith('loop', { flowName: 'x' })).map((i) => i.path)).toEqual(['nodes.1.config.flowName']); + // Presence and values keep `parsedWhen`: a marker loop with no collection, or a value the + // contract would refuse, is not judged. + expect(flowNodeConfigRefusals('loop', {})).toEqual([]); + expect(flowNodeConfigRefusals('loop', { collection: 5 })).toEqual([]); + }); + + it('a pre-conversion alias is converted first where the door converts, and refused at a direct parse', () => { + const config = { objectName: 'task', filters: { id: '{recordId}' }, fields: { title: 'X' } }; + expect(issuesOf(flowWith('update_record', config)).map((i) => i.path)).toEqual(['nodes.1.config.filters']); + const stack = { + manifest: { id: 'com.example.alias', name: 'alias', version: '1.0.0', type: 'app', namespace: 'als' }, + objects: [{ name: 'als_task', label: 'Task', fields: { title: { type: 'text', label: 'Title' } } }], + flows: [flowWith('update_record', { ...config, objectName: 'als_task' }, 'alias_flow')], + }; + expect(() => defineStack(stack as never)).not.toThrow(); + }); +}); + describe('what stays as it was (lit controls)', () => { it('CONTROL: the measured node without its extra key, and a valid subflow, parse clean', () => { for (const [type, config] of [['script', MEASURED], ['subflow', SUBFLOW]] as const) { @@ -129,23 +223,61 @@ describe('what stays as it was (lit controls)', () => { } }); - it('CONTROL: the key-judged builtins are exactly the schemaless ones with an executor contract', () => { + it('the key-judged builtins are every executor contract but try_catch, and the predicate says so', () => { const judged = [...getBuiltinNodeConfigContracts().keys()] .filter((type) => flowNodeConfigRefusals(type, { bogusKey: 1 }).some((r) => r.path === 'bogusKey')); - expect(judged.sort()).toEqual(['script', 'subflow']); - expect(judged.every((type) => Object.prototype.hasOwnProperty.call(SCHEMALESS_NODE_CONFIG_SCHEMAS, type))).toBe(true); + expect(judged.sort()).toEqual([ + 'create_record', 'delete_record', 'get_record', 'http', 'loop', 'map', 'notify', 'parallel', 'screen', + 'script', 'subflow', 'update_record', + ]); + // The predicate the descriptor walk reads names exactly the types the judge judges — one judge per type. + for (const type of getBuiltinNodeConfigContracts().keys()) { + expect(builtinNodeConfigKeysJudged(type), type).toBe(judged.includes(type)); + } + for (const type of ['try_catch', 'approval', 'decision', 'assignment', 'wait', 'some_plugin_node']) { + expect(builtinNodeConfigKeysJudged(type), type).toBe(false); + } + }); + + it('CONTROL: each judged type\'s valid config draws no refusal', () => { + for (const [type, config] of Object.entries(VALID)) { + expect(builtinNodeConfigKeysJudged(type), type).toBe(true); + expect(flowNodeConfigRefusals(type, config), type).toEqual([]); + } }); - it('CONTROL: a builtin with a descriptor configSchema keeps its undeclared key at registration', () => { + it('CONTROL: try_catch keeps its undeclared key at registration — its contract\'s retry strips one', () => { + const tryRegion = { nodes: [{ id: 'inner', type: 'assignment', label: 'A', config: { assignments: { a: 1 } } }], edges: [] }; + // The measured difference: the contract's `retry` accepts (strips) a key the descriptor closes. + expect(getBuiltinNodeConfigContracts().get('try_catch')!.schema + .safeParse({ try: tryRegion, retry: { maxRetries: 1, bogusKey: 1 } }).success).toBe(true); + for (const config of [{ try: tryRegion, bogusKey: 1 }, { try: tryRegion, retry: { maxRetries: 1, bogusKey: 1 } }]) { + expect(flowNodeConfigRefusals('try_catch', config)).toEqual([]); + } + }); + + it('CONTROL: a key at or under a region slot is the region\'s, never the container\'s', () => { + const node = { id: 'inner', type: 'assignment', label: 'A', config: { assignments: { a: 1 } } }; for (const [type, config] of [ - ['http', { url: 'https://example.test/hook', method: 'POST', bogusKey: 1 }], - ['create_record', { objectName: 'task', fields: { title: 'X' }, bogusKey: 1 }], - ['screen', { fields: [{ name: 'qty', visibleIf: 'x' }] }], + ['loop', { collection: '{rows}', body: { nodes: [node], edges: [], bogusKey: 1 } }], + ['loop', { collection: '{rows}', body: { nodes: [{ ...node, bogusKey: 1 }], edges: [] } }], + ['parallel', { branches: [{ name: 'a', nodes: [node], edges: [], bogusKey: 1 }, { name: 'b', nodes: [node], edges: [] }] }], ] as const) { - // The contract refuses it — the build arm holds back, registration judges it. + // The region contract refuses it — `validateControlFlow` names it at registration. expect(getBuiltinNodeConfigContracts().get(type)!.schema.safeParse(config).success, type).toBe(false); expect(flowNodeConfigRefusals(type, config), type).toEqual([]); - expect(issuesOf(flowWith(type, config)), type).toEqual([]); + } + }); + + it('CONTROL: a key inside a free-form map is author data', () => { + for (const [type, config] of [ + ['get_record', { objectName: 'task', filter: { status: 'stale', anythingAtAll: true } }], + ['create_record', { objectName: 'task', fields: { title: 'X', any_field: 1 } }], + ['http', { url: 'https://example.test/hook', headers: { 'X-Anything': 'y' } }], + ['screen', { objectName: 'task', defaults: { anything: 1 } }], + ['map', { collection: '{rows}', flowName: 'child_flow', input: { anything: 1 } }], + ] as const) { + expect(flowNodeConfigRefusals(type, config), type).toEqual([]); } }); @@ -174,6 +306,7 @@ describe('every door that parses a flow refuses it', () => { const refused = flowWith('script', { ...MEASURED, bogusKey: 1 }, 'key_refused'); const subflowRefused = flowWith('subflow', { ...SUBFLOW, bogusKey: 1 }, 'key_subflow'); const accepted = flowWith('script', MEASURED, 'key_ok'); + const notifyRefused = flowWith('notify', { ...NOTIFY, bogusKey: 1 }, 'key_notify'); it('defineStack wraps the refusal in its ADR-0112 envelope, at flows.1.nodes.1.config.bogusKey', () => { let refusal: { code?: unknown; status?: unknown; issues?: Array<{ path: unknown[]; code: string }> } | undefined; @@ -220,6 +353,14 @@ describe('every door that parses a flow refuses it', () => { const ok = ArtifactStagePackageBodySchema.safeParse({ ...body, flows: [accepted] }); expect(ok.success, JSON.stringify(ok.error?.issues ?? [])).toBe(true); }); + it('ObjectStackDefinitionSchema and the save door\'s flow type schema refuse the remainder\'s notify node', () => { + const r = ObjectStackDefinitionSchema.safeParse(stackWith([accepted, notifyRefused])); + expect(r.success).toBe(false); + expect(r.success ? [] : r.error.issues.map((i) => i.path.join('.'))).toEqual(['flows.1.nodes.1.config.bogusKey']); + const schema = getMetadataTypeSchema('flow') as unknown as typeof FlowSchema; + const saved = schema.safeParse(notifyRefused); + expect(saved.success ? [] : saved.error.issues.map((i) => i.path.join('.'))).toEqual(['nodes.1.config.bogusKey']); + }); }); describe('the ADR-0087 ledger', () => { @@ -234,4 +375,11 @@ describe('the ADR-0087 ledger', () => { it('names the step-18 rationale fragment for it', () => { expect(MIGRATIONS_BY_MAJOR[18]!.rationale).toContain(ENTRY_ID); }); + + it('registers the remainder\'s D3 entry at protocol 18, with no D2 conversion, and its rationale fragment', () => { + const entries = MIGRATIONS_BY_MAJOR[18]!.semantic.filter((e) => e.id === REMAINDER_ENTRY_ID); + expect(entries).toHaveLength(1); + expect(entries[0]!.conversionIds ?? []).toEqual([]); + expect(MIGRATIONS_BY_MAJOR[18]!.rationale).toContain(REMAINDER_ENTRY_ID); + }); }); diff --git a/packages/spec/src/automation/flow-decision-branch-expression-absent.test.ts b/packages/spec/src/automation/flow-decision-branch-expression-absent.test.ts index 4f9c3359dab..94ed7a814ac 100644 --- a/packages/spec/src/automation/flow-decision-branch-expression-absent.test.ts +++ b/packages/spec/src/automation/flow-decision-branch-expression-absent.test.ts @@ -44,7 +44,7 @@ const decision = (...branches: Node[]): Node => ({ const loopAround = (inner: Node): Node => ({ id: 'sweep', type: 'loop', label: 'Sweep', - config: { collection: '{items}', itemVariable: 'item', body: { nodes: [inner], edges: [] } }, + config: { collection: '{items}', iteratorVariable: 'item', body: { nodes: [inner], edges: [] } }, }); function issuesOf(flow: unknown) { diff --git a/packages/spec/src/automation/flow-node-config-refusals.ts b/packages/spec/src/automation/flow-node-config-refusals.ts index 8adcb8e2a7d..12eb3741533 100644 --- a/packages/spec/src/automation/flow-node-config-refusals.ts +++ b/packages/spec/src/automation/flow-node-config-refusals.ts @@ -418,20 +418,22 @@ export function builtinNodeConfigKeysJudged(nodeType: string): boolean { } /** - * [#21982] Does an `unrecognized_keys` issue sit INSIDE an ADR-0031 region - * rather than on the region object itself? The issue's `path` is the object - * that holds the key: `[]` for the config, `['body']` / `['try']` for a region - * object (arity `one`), `['branches', 0]` for one of a `many` slot's regions. - * A key on the region object is the container's, judged here like any other - * config key; anything deeper — a region node's or edge's own key — is the - * region's, refused at `registerFlow` by `validateControlFlow` and never - * re-reported against the container (the region policy `flow.zod.ts` states). + * [#21982] Does an `unrecognized_keys` issue sit at or under an ADR-0031 + * region slot — on a region object (`body`, `try`, `catch`, a `branches` + * element) or deeper, on a region node or edge? The issue's `path` is the + * object that holds the key, so `[]` is the config itself and `['body']` the + * loop's region object. A region's shape is `validateControlFlow`'s, as the + * value arm's own carve-out says ({@link builtinValueJudged}): it parses each + * region against the strict region contract at `registerFlow` and refuses an + * undeclared key on the region object and on its nodes and edges alike + * (measured: `try_catch 'n1' try: invalid region — Unrecognized key(s) on this + * control-flow region`), while the flow parse leaves a refused region raw for + * it to name (the region policy `flow.zod.ts` states). So such a key has its + * judge, one door later, and this arm does not add a second. */ -function unknownKeyInsideRegion(nodeType: string, path: ReadonlyArray): boolean { +function unknownKeyUnderRegionSlot(nodeType: string, path: ReadonlyArray): boolean { if (path.length === 0) return false; - const slot = (FLOW_REGION_SLOTS_BY_TYPE.get(nodeType) ?? []).find((s) => s.key === path[0]); - if (!slot) return false; - return path.length > (slot.arity === 'many' ? 2 : 1); + return (FLOW_REGION_SLOTS_BY_TYPE.get(nodeType) ?? []).some((slot) => slot.key === path[0]); } /** @@ -584,16 +586,17 @@ function unrecognizedKeysOf(issue: { readonly code: string }): readonly string[] * {@link builtinNodeConfigKeysJudged} judges (all but `try_catch`) → * `node-config-refused-by-contract`, anchored at the key, one refusal per * undeclared key (`bogusKey` on a `notify`; `fields[0].visibleIf` on a - * `screen` field; `body.bogusKey` on a `loop`'s region object), in the + * `screen` field; `flowName` on a body-less `loop`), in the * contract's own words — its prescription for a known slip included * (`fieldValues` → `fields`, `bulk` → `multi`, `visibleIf` → * `visibleWhen`, a `subflow` `timeoutMs` that belongs on the node) — and * closed with the rename-or-remove remedy. Each executor parses its strict * contract before anything else, so the run refuses such a node; this arm * is the one judge of it at every door, and `registerFlow`'s descriptor - * walk stands aside for these types. A key INSIDE a region — a region - * node's or edge's own key — is the region's (`validateControlFlow`), and - * a tombstoned key (a `retiredKey()`) keeps the path it had. + * walk stands aside for these types. A key at or under a region slot — on + * the region object, or a region node's or edge's own key — is the + * region's (`validateControlFlow`, at `registerFlow`), and a tombstoned + * key (a `retiredKey()`) keeps the path it had. * * Where the build cannot read the config whole, it reads only what is sound, * and each such type is named here, not skipped in silence: @@ -611,8 +614,7 @@ function unrecognizedKeysOf(issue: { readonly code: string }): readonly string[] * `try_catch` (`try`, `catch`) — hold regions, judged as graphs of their * own; a container is judged on the keys beside them (`try_catch`'s * `errorVariable` and `retry`), so `parallel`, whose one key is its region - * slot, is judged for presence and for the keys on each region object - * alone. + * slot, is judged for presence alone. * * A key only the conversion layer spells canonically (`object` → * `objectName`, `flow` → `flowName`, …) is judged AFTER the conversion at @@ -708,9 +710,8 @@ export function flowNodeConfigRefusals(nodeType: string, config: unknown): FlowN // An unknown key's issue sits on the object that holds it (the config // itself for a top-level key, so its `path` is empty): anchor one // refusal at each key the author wrote, with the contract's sentence. - // A key inside a region (a region node's or edge's own key) is the - // region's, never the container's. - const keys = unknownKeyInsideRegion(nodeType, issue.path) ? [] : unrecognizedKeysOf(issue); + // A key at or under a region slot is the region's (`validateControlFlow`). + const keys = unknownKeyUnderRegionSlot(nodeType, issue.path) ? [] : unrecognizedKeysOf(issue); for (const unknownKey of keys) { const key = ledgerPathOf([...issue.path, unknownKey]); if (seen.has(key)) continue; diff --git a/packages/spec/src/automation/flow-predicate-slot-blank.test.ts b/packages/spec/src/automation/flow-predicate-slot-blank.test.ts index 1c460378efc..ac938794680 100644 --- a/packages/spec/src/automation/flow-predicate-slot-blank.test.ts +++ b/packages/spec/src/automation/flow-predicate-slot-blank.test.ts @@ -57,7 +57,7 @@ const loopAround = (inner: Node): Node => ({ id: 'sweep', type: 'loop', label: 'Sweep', - config: { collection: '{items}', itemVariable: 'item', body: { nodes: [inner], edges: [] } }, + config: { collection: '{items}', iteratorVariable: 'item', body: { nodes: [inner], edges: [] } }, }); function issuesOf(flow: unknown) { diff --git a/packages/spec/src/migrations/entries/semantic/18.flow-builtin-node-config-undeclared-keys-refused.ts b/packages/spec/src/migrations/entries/semantic/18.flow-builtin-node-config-undeclared-keys-refused.ts index c7aa1ffb179..f4500923558 100644 --- a/packages/spec/src/migrations/entries/semantic/18.flow-builtin-node-config-undeclared-keys-refused.ts +++ b/packages/spec/src/migrations/entries/semantic/18.flow-builtin-node-config-undeclared-keys-refused.ts @@ -24,10 +24,11 @@ export const entry: SemanticMigration = { + 'flow node whose config carries a key its executor contract does not declare — a typo (titl), a key ' + 'the walk at registration already named (fieldValues on a write node, bulk on update_record, visibleIf ' + 'on a screen field), a key copied from another node type (outputVariable on an http node, flowName on a ' - + 'loop), or a key nothing reads (bogusKey) — at the config itself, on a screen field or one of its ' - + 'options, on a loop body region object or on a parallel branch, a body-less legacy loop included. ' - + 'Never a key inside a free-form map (a filter, fields, headers, defaults, input, payload or templateData ' - + 'key is author data), never a key on a region node or edge (the region check owns those), and never a ' + + 'loop), or a key nothing reads (bogusKey) — at the config itself, or on a screen field or one of its ' + + 'options, a body-less legacy loop included. Never a key inside a free-form map (a filter, fields, ' + + 'headers, defaults, input, payload or templateData key is author data), never a key on a region object ' + + '(a loop body, a parallel branch) or on its nodes and edges (the region check at registration owns ' + + 'those), and never a ' + 'try_catch key, which registration keeps judging against its descriptor. Reachable wherever a flow is ' + 'authored or stored: defineStack({ flows }) sources, defineFlow(), an exported stack passed to ' + 'objectstack validate or objectstack compile, a flow saved from the Studio flow designer, and a flow row ' diff --git a/packages/spec/src/migrations/registry.ts b/packages/spec/src/migrations/registry.ts index 493f0d01c8d..bca73937942 100644 --- a/packages/spec/src/migrations/registry.ts +++ b/packages/spec/src/migrations/registry.ts @@ -13474,10 +13474,11 @@ const step18: MigrationStep = { + 'flow node whose config carries a key its executor contract does not declare — a typo (titl), a key ' + 'the walk at registration already named (fieldValues on a write node, bulk on update_record, visibleIf ' + 'on a screen field), a key copied from another node type (outputVariable on an http node, flowName on a ' - + 'loop), or a key nothing reads (bogusKey) — at the config itself, on a screen field or one of its ' - + 'options, on a loop body region object or on a parallel branch, a body-less legacy loop included. ' - + 'Never a key inside a free-form map (a filter, fields, headers, defaults, input, payload or templateData ' - + 'key is author data), never a key on a region node or edge (the region check owns those), and never a ' + + 'loop), or a key nothing reads (bogusKey) — at the config itself, or on a screen field or one of its ' + + 'options, a body-less legacy loop included. Never a key inside a free-form map (a filter, fields, ' + + 'headers, defaults, input, payload or templateData key is author data), never a key on a region object ' + + '(a loop body, a parallel branch) or on its nodes and edges (the region check at registration owns ' + + 'those), and never a ' + 'try_catch key, which registration keeps judging against its descriptor. Reachable wherever a flow is ' + 'authored or stored: defineStack({ flows }) sources, defineFlow(), an exported stack passed to ' + 'objectstack validate or objectstack compile, a flow saved from the Studio flow designer, and a flow row ' From 82d14dfac0246f969e6b5af592144ea1fd9327db Mon Sep 17 00:00:00 2001 From: Claude Date: Thu, 8 Oct 2026 13:20:15 +0000 Subject: [PATCH 03/10] test(service-automation): re-point the descriptor walk's pins to the spec refusal; keep plugin and try_catch cases on the walk (wip) Claude-Session: https://claude.ai/code/session_01DhTqaEHqPVSVnAkjG3jywn Co-authored-by: Claude --- .../src/builtin/config-unknown-keys.test.ts | 199 ++++++++++++------ .../decision-branch-expression-absent.test.ts | 2 +- .../src/nested-region-parity.test.ts | 40 +++- .../src/predicate-slot-blank.test.ts | 2 +- 4 files changed, 172 insertions(+), 71 deletions(-) diff --git a/packages/services/service-automation/src/builtin/config-unknown-keys.test.ts b/packages/services/service-automation/src/builtin/config-unknown-keys.test.ts index daf797380f8..44b548e004b 100644 --- a/packages/services/service-automation/src/builtin/config-unknown-keys.test.ts +++ b/packages/services/service-automation/src/builtin/config-unknown-keys.test.ts @@ -10,17 +10,25 @@ * the only symptom was a feature that quietly did not happen. That is the * diagnostic vacuum that made #3528 take three passes and two wrong diagnoses. * - * #4059 turned that into a warning while the #4045 reconciliation closed the - * read-but-undeclared population; #4277 tightens it into a rejection. These - * tests pin both halves of the tightened behavior: registration THROWS with an - * actionable prescription (path, declared set, did-you-mean, per-key - * tombstones), AND the deliberate exemptions still register — `assignment` - * (author-named keys), keyValue-map keys (author data), schemaless types - * (nothing declared ⇒ nothing undeclared). + * [#21982] One judge per node type. A builtin's undeclared key is the spec's: + * `flowNodeConfigRefusals` refuses it inside the `FlowSchema.parse` + * `registerFlow` makes first — the same verdict `objectstack validate`, + * `objectstack compile` and the save door give — for every type + * `builtinNodeConfigKeysJudged` names, and the descriptor walk + * (`validateNodeConfigKeys`) stands aside for those types. The walk's pins are + * re-pointed here, not dropped: the key, its location, the prescription for a + * known slip and a rename-or-remove remedy all reach the author from the spec + * refusal. The walk keeps `try_catch` (its contract's `retry` strips an unknown + * key where its descriptor closes it) and every PLUGIN node type, and its own + * prescriptions are pinned on those. The deliberate exemptions still register: + * `assignment` (author-named keys), keyValue-map keys (author data), + * `decision` (no executor contract). */ import { describe, it, expect } from 'vitest'; +import { builtinNodeConfigKeysJudged, defineActionDescriptor, flowNodeConfigRefusals } from '@objectstack/spec/automation'; import { AutomationEngine } from '../engine.js'; +import type { NodeExecutor } from '../engine.js'; import { installBuiltinNodes } from './index.js'; function recordingLogger() { @@ -33,9 +41,21 @@ function recordingLogger() { return { logger, warnings }; } +/** A plugin node type the spec knows nothing about: its descriptor declares `count` and `label`. */ +const STAMP = 'test_stamp'; +const stampExecutor: NodeExecutor = { + type: STAMP, + descriptor: defineActionDescriptor({ + type: STAMP, version: '1.0.0', name: 'Stamp', category: 'custom', paradigms: ['flow'], source: 'plugin', + configSchema: { type: 'object', properties: { count: { type: 'number' }, label: { type: 'string' } } }, + }), + async execute() { return { success: true }; }, +}; + function engineWith(logger: any) { const engine = new AutomationEngine(logger); installBuiltinNodes(engine, { logger, getService() { throw new Error('none'); } } as any); + engine.registerNodeExecutor(stampExecutor); return engine; } @@ -55,93 +75,142 @@ function flowWith(type: string, config: Record) { }; } -/** Register and return the rejection message (fails the test if it registers). */ -function rejectionOf(engine: AutomationEngine, type: string, config: Record): string { +/** Register and return what it threw (fails the test if it registers). */ +function rejectionOf(engine: AutomationEngine, type: string, config: Record): Error & { issues?: any[] } { try { engine.registerFlow('f', flowWith(type, config)); } catch (err) { - return (err as Error).message; + return err as Error & { issues?: any[] }; } throw new Error(`flow with ${type} config ${JSON.stringify(config)} should have been rejected`); } -describe('unknown node config keys are rejected (#4277)', () => { +/** The spec refusal `registerFlow` met: every issue's path, with the judge's own refusal beside it. */ +function specRefusalsOf(engine: AutomationEngine, type: string, config: Record) { + const err = rejectionOf(engine, type, config); + expect(err.issues, 'the FlowSchema parse, not the descriptor walk, refused it').toBeDefined(); + const judged = flowNodeConfigRefusals(type, config); + return (err.issues ?? []).map((issue: any, i: number) => ({ + path: issue.path.join('.'), + message: issue.message as string, + code: judged[i]?.code, + judgedMessage: judged[i]?.message, + })); +} + +describe('a builtin\'s undeclared key is the spec\'s, met at registration (#4277 re-pointed, #21982)', () => { it('rejects a misspelled key inside the field repeater, locating the exact element', async () => { const { logger } = recordingLogger(); const engine = engineWith(logger); // The typo lives INSIDE the field repeater — where the real #3528 one did. - const msg = rejectionOf(engine, 'screen', { - fields: [{ name: 'opportunityName', required: true, visibleIf: 'createOpportunity == true' }], - }); - - expect(msg).toContain("node 'n1'"); - expect(msg).toContain('(screen)'); + const config = { fields: [{ name: 'opportunityName', required: true, visibleIf: 'createOpportunity == true' }] }; + const [refusal, ...rest] = specRefusalsOf(engine, 'screen', config); + expect(rest).toEqual([]); // Located to the exact element, so the author knows WHICH field. - expect(msg).toContain('config.fields[0].visibleIf'); - // The load-bearing half for an agent author: the correct key is named. Note - // it comes from the DECLARED SET, not the suggestion — `visibleIf` → - // `visibleWhen` is edit-distance 4 against `nearestName`'s threshold of 3, - // so this exact typo gets no did-you-mean. Printing the declared set is - // what makes the diagnostic actionable regardless. - // #17306 added `min`/`max`/`inlineHelpText`/`reference`; the enumeration is - // pinned in full rather than sampled, so a key that arrives or vanishes on - // this surface has to be acknowledged here. - expect(msg).toContain( - 'Declared here: name, label, type, required, options, defaultValue, placeholder, ' - + 'min, max, inlineHelpText, reference, visibleWhen.', - ); - // …and this particular key has a documented incident, so it also carries - // its tombstone (the UNKNOWN_KEY_GUIDANCE pattern). - expect(msg).toContain('visibleWhen'); - expect(msg).toContain('blocks the screen from ever being submitted'); + expect(refusal!.path).toBe('nodes.1.config.fields.0.visibleIf'); + expect(refusal!.code).toBe('node-config-refused-by-contract'); + expect(refusal!.message).toBe(refusal!.judgedMessage); + // The load-bearing half for an agent author: the correct key is named, by + // the screen field contract's own prescription for this exact typo. + expect(refusal!.message).toContain('`visibleWhen`'); // The flow is NOT registered. await expect(engine.getFlow('f')).resolves.toBeNull(); }); it('adds a did-you-mean when the typo IS within edit distance', () => { - const { logger } = recordingLogger(); - const engine = engineWith(logger); - - const msg = rejectionOf(engine, 'screen', { titl: 'Details' }); - expect(msg).toContain('did you mean `title`?'); - // …and still lists the declared set alongside it. - expect(msg).toContain('Declared here:'); + const engine = engineWith(recordingLogger().logger); + const [refusal] = specRefusalsOf(engine, 'screen', { titl: 'Details' }); + expect(refusal!.path).toBe('nodes.1.config.titl'); + expect(refusal!.message).toContain('`title`'); }); it('carries the fieldValues → fields tombstone on the CRUD write map', () => { - const { logger } = recordingLogger(); - const engine = engineWith(logger); - - const msg = rejectionOf(engine, 'create_record', { - objectName: 'crm_lead', - fieldValues: { name: 'x' }, - }); - expect(msg).toContain('unknown config key `fieldValues`'); + const engine = engineWith(recordingLogger().logger); + const [refusal] = specRefusalsOf(engine, 'create_record', { objectName: 'crm_lead', fieldValues: { name: 'x' } }); + expect(refusal!.path).toBe('nodes.1.config.fieldValues'); // The tombstone names the mechanism, not just the nearest key. - expect(msg).toContain('The write map is `fields`'); + expect(refusal!.message).toContain('The write map is `fields`'); }); it('reports every undeclared key in ONE rejection, not just the first', () => { - const { logger } = recordingLogger(); - const engine = engineWith(logger); + const engine = engineWith(recordingLogger().logger); + const refusals = specRefusalsOf(engine, 'screen', { hideWhen: 'x', submitLabel: 'Go' }); + expect(refusals.map((r) => r.path).sort()).toEqual(['nodes.1.config.hideWhen', 'nodes.1.config.submitLabel']); + }); - const msg = rejectionOf(engine, 'screen', { hideWhen: 'x', submitLabel: 'Go' }); - expect(msg).toContain('hideWhen'); - expect(msg).toContain('submitLabel'); - expect(msg).toContain('2 undeclared config key(s)'); + it('a key with no did-you-mean still carries the rename-or-remove remedy', () => { + const engine = engineWith(recordingLogger().logger); + const [refusal] = specRefusalsOf(engine, 'screen', { totallyMadeUp: 42 }); + expect(refusal!.path).toBe('nodes.1.config.totallyMadeUp'); + expect(refusal!.message).not.toContain('did you mean'); + expect(refusal!.message).toMatch(/Rename the key .* or remove it/); }); - it('the rejection carries the declare-it prescription for genuinely-read keys', () => { - const { logger } = recordingLogger(); - const engine = engineWith(logger); + it('a body-less legacy loop with an undeclared key is still refused at registration — registerFlow widens nowhere', async () => { + const engine = engineWith(recordingLogger().logger); + const [refusal] = specRefusalsOf(engine, 'loop', { collection: 'rows', bogusKey: 1 }); + expect(refusal!.path).toBe('nodes.1.config.bogusKey'); + await expect(engine.getFlow('f')).resolves.toBeNull(); + // CONTROL: the same legacy loop without the key registers. + expect(() => engine.registerFlow('f', flowWith('loop', { collection: 'rows' }))).not.toThrow(); + }); + + it('the descriptor walk stands aside for every type the spec judges, and keeps the rest', () => { + const engine = engineWith(recordingLogger().logger); + const walk = (type: string, config: Record) => () => + (engine as any).validateNodeConfigKeys('f', flowWith(type, config)); + // One judge per type: the spec already refused these in `FlowSchema.parse`. + for (const [type, config] of [ + ['screen', { fields: [{ name: 'a', visibleIf: 'x' }] }], + ['notify', { recipients: 'u1', title: 'T', bogusKey: 1 }], + ['loop', { collection: 'rows', bogusKey: 1 }], + ] as const) { + expect(builtinNodeConfigKeysJudged(type), type).toBe(true); + expect(walk(type, config), type).not.toThrow(); + } + // The walk keeps try_catch and plugin types. + expect(builtinNodeConfigKeysJudged('try_catch')).toBe(false); + expect(walk('try_catch', { try: { nodes: [], edges: [] }, bogusKey: 1 })).toThrow(/undeclared config key/); + expect(builtinNodeConfigKeysJudged(STAMP)).toBe(false); + expect(walk(STAMP, { count: 1, bogusKey: 1 })).toThrow(/undeclared config key/); + }); +}); - const msg = rejectionOf(engine, 'screen', { totallyMadeUp: 42 }); - expect(msg).not.toContain('did you mean'); +describe('the descriptor walk keeps try_catch and plugin node types, with its own prescriptions', () => { + it('try_catch: a key its descriptor closes on `retry` — one the contract would strip — is refused, located', async () => { + const engine = engineWith(recordingLogger().logger); + const tryRegion = { nodes: [{ id: 'a', type: 'assignment', label: 'A', config: { assignments: { x: 1 } } }], edges: [] }; + const err = rejectionOf(engine, 'try_catch', { try: tryRegion, retry: { maxRetries: 1, bogusKey: 1 } }); + expect(err.issues).toBeUndefined(); + expect(err.message).toContain("node 'n1' (try_catch)"); + expect(err.message).toContain('unknown config key `bogusKey` at config.retry.bogusKey'); + expect(err.message).toContain('Declared here: maxRetries, backoffMs, backoffMultiplier, maxRetryDelayMs, jitter.'); + expect(err.message).toContain('(rename or remove the key)'); + await expect(engine.getFlow('f')).resolves.toBeNull(); + }); + + it('a plugin node type: did-you-mean, the declared set, and the declare-it prescription', () => { + const engine = engineWith(recordingLogger().logger); + const err = rejectionOf(engine, STAMP, { coutn: 1 }); + expect(err.issues).toBeUndefined(); + expect(err.message).toContain(`node 'n1' (${STAMP}): unknown config key \`coutn\` at config.coutn`); + expect(err.message).toContain('did you mean `count`?'); + expect(err.message).toContain('Declared here: count, label.'); // The way OUT for a custom executor whose schema lags its reads. - expect(msg).toContain("declare it on the node type's descriptor configSchema"); + expect(err.message).toContain("declare it on the node type's descriptor configSchema"); }); + it('a plugin node type: every undeclared key in ONE rejection', () => { + const engine = engineWith(recordingLogger().logger); + const err = rejectionOf(engine, STAMP, { hideWhen: 'x', submitLabel: 'Go' }); + expect(err.message).toContain('hideWhen'); + expect(err.message).toContain('submitLabel'); + expect(err.message).toContain('2 undeclared config key(s)'); + }); +}); + +describe('what still registers (controls)', () => { it('registers a fully declared config without complaint', async () => { const { logger, warnings } = recordingLogger(); const engine = engineWith(logger); @@ -174,8 +243,8 @@ describe('unknown node config keys are rejected (#4277)', () => { }); it('stays quiet for a node type that publishes no configSchema', () => { - // `decision` / `script` are deliberately schemaless (config-schemas.test.ts): - // nothing is declared, so nothing can be undeclared. + // `decision` is deliberately schemaless (config-schemas.test.ts) and has no + // executor contract either: nothing is declared, so nothing can be undeclared. const { logger } = recordingLogger(); const engine = engineWith(logger); @@ -184,7 +253,7 @@ describe('unknown node config keys are rejected (#4277)', () => { }); it('does not flag the keys of a keyValue map — those are author data', () => { - // The walk descends where the schema declares structure and STOPS at a + // Both judges descend where the contract declares structure and STOP at a // free-form map: `filter: { status: 'stale' }` keys are data, not config keys. const { logger } = recordingLogger(); const engine = engineWith(logger); diff --git a/packages/services/service-automation/src/decision-branch-expression-absent.test.ts b/packages/services/service-automation/src/decision-branch-expression-absent.test.ts index 4e325fe5a2b..10fb6c99c0f 100644 --- a/packages/services/service-automation/src/decision-branch-expression-absent.test.ts +++ b/packages/services/service-automation/src/decision-branch-expression-absent.test.ts @@ -86,7 +86,7 @@ describe('registerFlow refuses a decision branch with no `expression` (#19961)', it('refuses an absent branch inside an ADR-0031 region body, anchored where the author wrote it', () => { const refusal = refusalOf(new AutomationEngine(silentLogger), flowWith({ id: 'sweep', type: 'loop', label: 'Sweep', - config: { collection: '{items}', itemVariable: 'item', body: { nodes: [decision({ label: 'y' })], edges: [] } }, + config: { collection: '{items}', iteratorVariable: 'item', body: { nodes: [decision({ label: 'y' })], edges: [] } }, })); expect(refusal?.issues?.map((i) => [i.code, i.path])).toEqual([ ['custom', ['nodes', 1, 'config', 'body', 'nodes', 0, 'config', 'conditions', 0, 'expression']], diff --git a/packages/services/service-automation/src/nested-region-parity.test.ts b/packages/services/service-automation/src/nested-region-parity.test.ts index 645ac935b2f..502e6d183d6 100644 --- a/packages/services/service-automation/src/nested-region-parity.test.ts +++ b/packages/services/service-automation/src/nested-region-parity.test.ts @@ -22,6 +22,7 @@ import type { NodeExecutor } from './engine.js'; import { registerLoopNode } from './builtin/loop-node.js'; import { registerLogicNodes } from './builtin/logic-nodes.js'; import { registerCrudNodes } from './builtin/crud-nodes.js'; +import { defineActionDescriptor } from '@objectstack/spec/automation'; function silentLogger() { return { info() {}, warn() {}, error() {}, debug() {}, child() { return silentLogger(); } } as any; @@ -255,15 +256,30 @@ describe('#4389 — registration validators cover region graphs', () => { }); describe('validateNodeConfigKeys (hard-fail)', () => { + // [#21982] A builtin's undeclared key is the spec's now (`FlowSchema.parse`, + // `builtinNodeConfigKeysJudged`), and the descriptor walk stands aside for + // it. The walk keeps the PLUGIN node types — so its region coverage (#4389) + // is pinned on one: `region_stamp` declares `count`, and `countz` is the + // #4277 typo class. + const STAMP = 'region_stamp'; + const stampExecutor: NodeExecutor = { + type: STAMP, + descriptor: defineActionDescriptor({ + type: STAMP, version: '1.0.0', name: 'Region stamp', category: 'custom', paradigms: ['flow'], source: 'plugin', + configSchema: { type: 'object', properties: { count: { type: 'number' } } }, + }), + async execute() { return { success: true }; }, + }; + let engine: AutomationEngine; beforeEach(() => { engine = new AutomationEngine(silentLogger()); registerLoopNode(engine, ctx()); registerCrudNodes(engine, ctx()); + engine.registerNodeExecutor(stampExecutor); }); - // `fieldz` is not on the `create_record` descriptor — the #4277 typo class. - const typoNode = [{ id: 'w', type: 'create_record', label: 'W', config: { objectName: 'lead', fieldz: { a: 1 } } }]; + const typoNode = [{ id: 'w', type: STAMP, label: 'W', config: { count: 1, countz: 2 } }]; it('rejects an undeclared config key at the top level (unchanged)', () => { expect(() => engine.registerFlow('sweep', flowWith(false, typoNode))).toThrow(/undeclared config key/); @@ -281,14 +297,30 @@ describe('#4389 — registration validators cover region graphs', () => { } catch (err) { const msg = (err as Error).message; expect(msg).toMatch(/1 undeclared config key/); - expect(msg.match(/`fieldz`/g)).toHaveLength(1); + expect(msg.match(/`countz`/g)).toHaveLength(1); } }); it('leaves a correct region alone', () => { - const ok = [{ id: 'w', type: 'create_record', label: 'W', config: { objectName: 'lead', fields: { a: 1 } } }]; + const ok = [{ id: 'w', type: STAMP, label: 'W', config: { count: 1 } }]; expect(() => engine.registerFlow('sweep', flowWith(true, ok))).not.toThrow(); }); + + it('[#21982] a builtin\'s undeclared key in a loop body is the spec\'s, refused once at the path the author wrote', () => { + const builtinTypo = [{ id: 'w', type: 'create_record', label: 'W', config: { objectName: 'lead', fieldz: { a: 1 } } }]; + for (const nested of [false, true]) { + let issues: Array<{ path: PropertyKey[] }> = []; + try { + engine.registerFlow('sweep', flowWith(nested, builtinTypo)); + throw new Error('expected a rejection'); + } catch (err) { + issues = (err as { issues?: Array<{ path: PropertyKey[] }> }).issues ?? []; + } + expect(issues.map((i) => i.path.join('.')), String(nested)).toEqual([ + nested ? 'nodes.1.config.body.nodes.0.config.fieldz' : 'nodes.1.config.fieldz', + ]); + } + }); }); describe('unknown node types (soft-fail)', () => { diff --git a/packages/services/service-automation/src/predicate-slot-blank.test.ts b/packages/services/service-automation/src/predicate-slot-blank.test.ts index 8b0f8bcbe00..d9043c3b7b7 100644 --- a/packages/services/service-automation/src/predicate-slot-blank.test.ts +++ b/packages/services/service-automation/src/predicate-slot-blank.test.ts @@ -57,7 +57,7 @@ const screen = (visibleWhen: unknown): Node => ({ const loopAround = (inner: Node): Node => ({ id: 'sweep', type: 'loop', label: 'Sweep', - config: { collection: '{items}', itemVariable: 'item', body: { nodes: [inner], edges: [] } }, + config: { collection: '{items}', iteratorVariable: 'item', body: { nodes: [inner], edges: [] } }, }); /** What `registerFlow` threw, or `undefined` when it registered. */ From d0511ae2145b4fdf0a03279fba1d587173a53be2 Mon Sep 17 00:00:00 2001 From: Claude Date: Thu, 8 Oct 2026 13:29:01 +0000 Subject: [PATCH 04/10] test(runtime,lint): the fakes' walk models a plugin type; a builtin's key is the spec's; loop fixture uses its declared key (wip) Claude-Session: https://claude.ai/code/session_01DhTqaEHqPVSVnAkjG3jywn Co-authored-by: Claude --- .../lint/src/validate-expressions.test.ts | 2 +- .../automation-put-post-error-parity.test.ts | 46 ++++++++++++++---- .../automation-register-error-class.test.ts | 48 +++++++++++++++---- 3 files changed, 75 insertions(+), 21 deletions(-) diff --git a/packages/lint/src/validate-expressions.test.ts b/packages/lint/src/validate-expressions.test.ts index 320e3db3fc7..3ae7b9afddc 100644 --- a/packages/lint/src/validate-expressions.test.ts +++ b/packages/lint/src/validate-expressions.test.ts @@ -4379,7 +4379,7 @@ describe('a blank string in a ledger predicate slot (#17493)', () => { it('reaches a `decision` inside an ADR-0031 region body', () => { const found = errorsOf(flowStack({ id: 'sweep', type: 'loop', - config: { collection: '{items}', itemVariable: 'item', body: { nodes: [decision(' ')], edges: [] } }, + config: { collection: '{items}', iteratorVariable: 'item', body: { nodes: [decision(' ')], edges: [] } }, })); expect(found).toHaveLength(1); expect(found[0].where).toContain("loop 'sweep' body"); 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 7296e9e65dc..b671128d5b2 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 @@ -44,10 +44,16 @@ 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). */ -// `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']; +/** + * [#21982] The node type the fake's #4277 walk judges: a PLUGIN type the spec + * knows nothing about, because the real walk (`validateNodeConfigKeys`) now + * judges only those (and `try_catch`). A builtin's undeclared key is refused + * one step earlier, by the real `FlowSchema.parse` this fake runs first + * (`builtinUndeclaredConfigKey` below). + */ +const STAMP = 'test_stamp'; +/** Config keys the fake's `test_stamp` descriptor declares (the #4277 legal set). */ +const STAMP_DECLARED_CONFIG_KEYS = ['count', 'label']; /** * The #4277 refusal, reproduced from `service-automation/src/engine.ts` @@ -58,7 +64,7 @@ function undeclaredConfigKeyRefusal(flowName: string, nodeId: string, nodeType: const violation = `node '${nodeId}' (${nodeType}): unknown config key \`${key}\` at config.${key}` + ` It is not declared by this node type's configSchema, so nothing reads it.` + - ` Declared here: ${NOTIFY_DECLARED_CONFIG_KEYS.join(', ')}.`; + ` Declared here: ${STAMP_DECLARED_CONFIG_KEYS.join(', ')}.`; return new Error( `Flow '${flowName}' rejected: 1 undeclared config key(s) (#4277).\n` + ` - ${violation}\n` + @@ -87,9 +93,9 @@ function makeDispatcher() { const parsed = FlowSchema.parse(definition) as { nodes?: Array> }; validateControlFlow(parsed as any); for (const node of parsed.nodes ?? []) { - if (node.type !== 'notify') continue; + if (node.type !== STAMP) continue; for (const key of Object.keys(node.config ?? {})) { - if (!NOTIFY_DECLARED_CONFIG_KEYS.includes(key)) { + if (!STAMP_DECLARED_CONFIG_KEYS.includes(key)) { throw undeclaredConfigKeyRefusal(String((definition as any)?.name), node.id, node.type, key); } } @@ -162,8 +168,16 @@ const BAD_BODIES = { config: { try: [], catch: { nodes: [], edges: [] } }, }], }, - /** 4 — a config key the node type's descriptor does not declare (#4277). */ + /** 4 — a config key a PLUGIN node type's descriptor does not declare (#4277). */ undeclaredConfigKey: { + ...WELL_FORMED, + nodes: [{ + id: 'n', type: STAMP, label: 'Stamp', + config: { count: 1, totallyBogusKey: 'oops' }, + }], + }, + /** 5 — [#21982] a config key a BUILTIN node's executor contract does not declare: the spec refuses it in the parse. */ + builtinUndeclaredConfigKey: { ...WELL_FORMED, nodes: [{ id: 'n', type: 'notify', label: 'Notify', @@ -248,14 +262,26 @@ describe('#8123 — POST and PUT agree on the class of an identical flow refusal expect(message).toContain('unknown config key `totallyBogusKey`'); expect(message).toContain('at config.totallyBogusKey'); expect(message).toContain("not declared by this node type's configSchema"); - expect(message).toContain(`Declared here: ${NOTIFY_DECLARED_CONFIG_KEYS.join(', ')}.`); - expect(message).toContain("node 'n' (notify)"); + expect(message).toContain(`Declared here: ${STAMP_DECLARED_CONFIG_KEYS.join(', ')}.`); + expect(message).toContain(`node 'n' (${STAMP})`); // …and the same substance reaches the field entry, not a stub. const fields = result.response.body.error.details.fields; expect(fields).toHaveLength(1); expect(fields[0]).toEqual({ field: '(body)', code: 'invalid_value', message }); }); + it('case "builtinUndeclaredConfigKey" (#21982): the spec\'s located refusal on PUT, the walk never consulted', async () => { + const { dispatcher } = makeDispatcher(); + const body = BAD_BODIES.builtinUndeclaredConfigKey; + const result: any = await putFlow(dispatcher, body.name, body); + + assertValidationEnvelope(result.response, 'PUT builtin undeclared config key'); + expect(result.response.body.error.message).not.toContain('#4277'); + const located = result.response.body.error.details.fields.filter((f: any) => f.field === 'nodes.0.config.totallyBogusKey'); + expect(located).toHaveLength(1); + expect(located[0].message).toContain('`totallyBogusKey`'); + }); + it('case "missingNodeLabel": the raw Zod issue array does not reach the wire on PUT either', async () => { const { dispatcher } = makeDispatcher(); const body = BAD_BODIES.missingNodeLabel; 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 b2cf8970e40..02533451c36 100644 --- a/packages/runtime/src/domains/automation-register-error-class.test.ts +++ b/packages/runtime/src/domains/automation-register-error-class.test.ts @@ -73,10 +73,16 @@ 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). */ -// `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']; +/** + * [#21982] The node type the fake's #4277 walk judges: a PLUGIN type the spec + * knows nothing about, because the real walk (`validateNodeConfigKeys`) now + * judges only those (and `try_catch`). A builtin's undeclared key is refused + * one step earlier, by the real `FlowSchema.parse` this fake runs first + * (`builtinUndeclaredConfigKey` below). + */ +const STAMP = 'test_stamp'; +/** Config keys the fake's `test_stamp` descriptor declares (the #4277 legal set). */ +const STAMP_DECLARED_CONFIG_KEYS = ['count', 'label']; /** * The #4277 refusal, reproduced from `service-automation/src/engine.ts` @@ -88,7 +94,7 @@ function undeclaredConfigKeyRefusal(flowName: string, nodeId: string, nodeType: const violation = `node '${nodeId}' (${nodeType}): unknown config key \`${key}\` at config.${key}` + ` It is not declared by this node type's configSchema, so nothing reads it.` + - ` Declared here: ${NOTIFY_DECLARED_CONFIG_KEYS.join(', ')}.`; + ` Declared here: ${STAMP_DECLARED_CONFIG_KEYS.join(', ')}.`; return new Error( `Flow '${flowName}' rejected: 1 undeclared config key(s) (#4277).\n` + ` - ${violation}\n` + @@ -116,9 +122,9 @@ function makeDispatcher(options?: { registerFlow?: (name: string, definition: un const parsed = FlowSchema.parse(definition) as { nodes?: Array> }; validateControlFlow(parsed as any); for (const node of parsed.nodes ?? []) { - if (node.type !== 'notify') continue; + if (node.type !== STAMP) continue; for (const key of Object.keys(node.config ?? {})) { - if (!NOTIFY_DECLARED_CONFIG_KEYS.includes(key)) { + if (!STAMP_DECLARED_CONFIG_KEYS.includes(key)) { throw undeclaredConfigKeyRefusal(String((definition as any)?.name), node.id, node.type, key); } } @@ -194,8 +200,16 @@ const BAD_BODIES = { config: { try: [], catch: { nodes: [], edges: [] } }, }], }, - /** 4 — a config key the node type's descriptor does not declare (#4277). */ + /** 4 — a config key a PLUGIN node type's descriptor does not declare (#4277). */ undeclaredConfigKey: { + ...WELL_FORMED, + nodes: [{ + id: 'n', type: STAMP, label: 'Stamp', + config: { count: 1, totallyBogusKey: 'oops' }, + }], + }, + /** 5 — [#21982] a config key a BUILTIN node's executor contract does not declare: the spec refuses it in the parse. */ + builtinUndeclaredConfigKey: { ...WELL_FORMED, nodes: [{ id: 'n', type: 'notify', label: 'Notify', @@ -310,13 +324,27 @@ describe('#8055 — a malformed flow definition is 400 VALIDATION_FAILED, not 50 expect(message).toContain('at config.totallyBogusKey'); expect(message).toContain("not declared by this node type's configSchema"); // The prescription — WHICH keys the author may use instead. - expect(message).toContain(`Declared here: ${NOTIFY_DECLARED_CONFIG_KEYS.join(', ')}.`); - expect(message).toContain("node 'n' (notify)"); + expect(message).toContain(`Declared here: ${STAMP_DECLARED_CONFIG_KEYS.join(', ')}.`); + expect(message).toContain(`node 'n' (${STAMP})`); // …and the same substance reaches the field entry, not a stub. expect(fields).toHaveLength(1); expect(fields[0]).toEqual({ field: '(body)', code: 'invalid_value', message }); }); + it('case 5 — [#21982] a builtin\'s undeclared config key: the spec\'s refusal, located, under the same honest status', async () => { + const { res, fields } = await expectValidationEnvelope( + BAD_BODIES.builtinUndeclaredConfigKey, 'builtin undeclared config key', + ); + + // One judge: the real `FlowSchema.parse` refused it, so the fake's walk never ran. + expect(res.body.error.message).not.toContain('#4277'); + // Located where the author wrote it, and named. + const located = fields.filter((f: any) => f.field === 'nodes.0.config.totallyBogusKey'); + expect(located).toHaveLength(1); + expect(located[0].message).toContain('`totallyBogusKey`'); + expect(JSON.stringify(res.body)).not.toContain('"keys"'); + }); + it('no malformed body reaches 500 by any route — no retry-on-5xx client is provoked', async () => { for (const [label, body] of Object.entries(BAD_BODIES)) { const { dispatcher } = makeDispatcher(); From fda538dff91eb5490b1d7442453b2c57e3b311a8 Mon Sep 17 00:00:00 2001 From: Claude Date: Thu, 8 Oct 2026 13:30:00 +0000 Subject: [PATCH 05/10] chore(changeset): 21982 builtin undeclared keys refused (wip) Claude-Session: https://claude.ai/code/session_01DhTqaEHqPVSVnAkjG3jywn Co-authored-by: Claude --- ...tin-node-config-undeclared-keys-refused.md | 48 +++++++++++++++++++ 1 file changed, 48 insertions(+) create mode 100644 .changeset/21982-flow-builtin-node-config-undeclared-keys-refused.md diff --git a/.changeset/21982-flow-builtin-node-config-undeclared-keys-refused.md b/.changeset/21982-flow-builtin-node-config-undeclared-keys-refused.md new file mode 100644 index 00000000000..2e8e4850fe4 --- /dev/null +++ b/.changeset/21982-flow-builtin-node-config-undeclared-keys-refused.md @@ -0,0 +1,48 @@ +--- +'@objectstack/spec': minor +'@objectstack/service-automation': patch +--- + +A key a builtin flow node's executor contract does not declare is refused at parse, with a location, on every builtin but `try_catch`: a `notify` `bogusKey`, a screen field's `visibleIf`, a `create_record` `fieldValues`, an `http` `outputVariable` and the like no longer pass `objectstack validate` and `objectstack compile` and then get the whole flow refused at registration. + +Clause-②: no (narrowing) + + + +**BREAKING**: an accept-set narrowing on a published authoring surface, shipped as `minor` under the launch-window convention for accept-set narrowings. + +**Why.** Each of these executors parses the node's `config` against a strict contract before it acts. The build doors' executor-contract arm judged required keys and present values on these types, but held key membership back, because `registerFlow`'s undeclared-key walk judged it against the node type descriptor's `configSchema`. So a `notify` node carrying `bogusKey` passed `FlowSchema.parse`, `objectstack validate` and `objectstack compile` (compile copied the key into `dist/objectstack.json`), and then registration refused the whole flow: at boot it was skipped with a warn, and a flow saved from Studio was stored and then silently not registered. + +**What is refused.** A `get_record`, `create_record`, `update_record`, `delete_record`, `notify`, `http`, `screen`, `map`, `loop` or `parallel` node, at any depth, whose config carries a key its executor contract does not declare: at the config itself, on a `screen` field, or on one of a field's `options`. That includes a body-less legacy `loop`, which is judged on key membership alone. The refusal is the existing closed-set code `node-config-refused-by-contract`, `params: { nodeType, key }`, one per undeclared key, anchored at the key (`nodes.N.config.bogusKey`, `nodes.N.config.fields.0.visibleIf`). It comes from the one judge `flowNodeConfigRefusals` that `FlowSchema.parse`, `AutomationEngine.registerFlow` (which parses first), `objectstack validate` and the metadata save door share. Its message carries the contract's own sentence, with the contract's prescription for a known slip (`fieldValues` → `fields`, `bulk` → `multi: true`, `visibleIf` → `visibleWhen`, a did-you-mean for a near miss), and closes with the remedy: rename the key to one the contract declares there, or remove it. The issue's `code` is `custom`. That covers `FlowSchema`, `defineFlow()`, `defineStack` (`STACK_SCHEMA_INVALID`, 422, at `flows.N.nodes.M.config.`), `os validate`, `os compile`, an artifact's parse, `registerFlow` and the save door. `script` and `subflow` keys were already refused this way. The new `builtinNodeConfigKeysJudged(nodeType)` export names exactly the builtins the spec judges. + +**`@objectstack/service-automation`.** `registerFlow`'s descriptor walk (`validateNodeConfigKeys`) stands aside for every type `builtinNodeConfigKeysJudged` names, so each node type has one judge. Before the move, the declared key sets were measured equal: on each of these types the descriptor's declared keys, at every position the walk descends to, equal the keys the contract accepts there. So registration refuses exactly what it refused before. The refusal now arrives as the parse's located issue instead of the walk's `Flow '…' rejected: N undeclared config key(s)` text. + +**What stays as it was.** + +- `try_catch`: its undeclared keys stay registration's, judged by the walk. Its contract's `retry` is the shared `RetryPolicySchema`, which strips an unknown key, while its descriptor closes `retry` to `maxRetries`, `backoffMs`, `backoffMultiplier`, `maxRetryDelayMs` and `jitter`. Moving the judge would have widened registration. +- Every plugin node type (the spec does not declare its contract): judged by the walk against its descriptor's `configSchema`, with the walk's own prescriptions. +- A key at or under a region slot (a `loop` body or a `parallel` branch, and their nodes and edges): judged at registration by the region check, as before. +- A key inside a free-form map (`filter`, `fields`, `headers`, `defaults`, `input`, `payload`, `templateData`): author data, never judged. +- `assignment`, whose top-level keys are the author's variable names, and `decision`, which parses no contract: unjudged. +- A spelling an ADR-0087 D2 conversion still rewrites at load (`object` and `filters` on a CRUD node; `to`, `subject`, `body` and `url` on a `notify`; `flow` on a `map`) is converted before the judge at every door that converts first (`defineStack`, `os validate`, `os compile`, `registerFlow`, the save door). Met by a direct `FlowSchema.parse` or `defineFlow()`, it is refused like any other undeclared key. + +## FROM → TO + +| you wrote | write instead | +|:--|:--| +| a typo of a declared key (`titl`, `outputVariabel`) | the declared key the refusal's did-you-mean names | +| `fieldValues` on `create_record` / `update_record` | `fields` | +| `bulk`, `all`, `multiple` or `options: { multi: true }` on `update_record` / `delete_record` | `multi: true` at the top level of `config` | +| `visibleIf` on a screen field | `visibleWhen` | +| `itemVariable` on a `loop` | `iteratorVariable` | +| `outputVariable` on an `http` node | delete it: the http executor binds no output variable | +| a key nothing reads | delete it | + +**The one-line fix: rename or delete the key the refusal names.** Registration already refused such a flow, so the fix changes nothing a running flow does. + +**Who is affected, measured.** At `fbcbcf124`, every builtin flow node authored in this repository's examples, platform objects, apps, scaffolding templates, skills and docs (95 nodes) carries only keys its contract declares. The Studio flow designer at the pinned objectui `a58626c88d` writes only declared keys whenever the engine publishes the node's descriptor. Its fallback form (while the descriptor list is loading, or when it cannot be read) offers `outputVariable` on an `http` node and the `url` alias on a `notify` node. Registration already refused the first and the conversion rewrites the second; the designer fix is tracked in objectui. hotcrm, deployed metadata and other repositories were not measured. + +### The kit + +- **The refusal.** The key half of the executor-contract arm of `flowNodeConfigRefusals` in `automation/flow-node-config-refusals.ts`, over every builtin in `getBuiltinNodeConfigContracts()` but `try_catch`, named by the new export `builtinNodeConfigKeysJudged`; no new code joins `FLOW_SLOT_REFUSAL_CODES`. +- **The ledger.** The D3 semantic entry `flow-builtin-node-config-undeclared-keys-refused` (protocol 18). No key is removed, so there is no tombstone, and there is no D2 conversion: the platform cannot know what an undeclared key was meant to be. From b9a3295d11f21eef58a7e651a20cfb5f45d1cbef Mon Sep 17 00:00:00 2001 From: Claude Date: Thu, 8 Oct 2026 13:42:26 +0000 Subject: [PATCH 06/10] chore(spec): regenerate api-surface and export-origins for builtinNodeConfigKeysJudged (wip) Claude-Session: https://claude.ai/code/session_01DhTqaEHqPVSVnAkjG3jywn Co-authored-by: Claude --- packages/spec/api-surface/automation.json | 1 + packages/spec/export-origins/automation.json | 1 + 2 files changed, 2 insertions(+) diff --git a/packages/spec/api-surface/automation.json b/packages/spec/api-surface/automation.json index 9c3731702f2..0e15e192afb 100644 --- a/packages/spec/api-surface/automation.json +++ b/packages/spec/api-surface/automation.json @@ -269,6 +269,7 @@ "WebhookTriggerType (type)", "analyzeRegion (function)", "approverTypeIsOrgScoped (function)", + "builtinNodeConfigKeysJudged (function)", "canonicalApproverType (function)", "collectFlowGraphs (function)", "defineActionDescriptor (function)", diff --git a/packages/spec/export-origins/automation.json b/packages/spec/export-origins/automation.json index f02aea90b14..e76469b9c77 100644 --- a/packages/spec/export-origins/automation.json +++ b/packages/spec/export-origins/automation.json @@ -263,6 +263,7 @@ "WebhookTriggerType": "src/automation/webhook.zod.ts#WebhookTriggerType (type)", "analyzeRegion": "src/automation/control-flow.zod.ts#analyzeRegion (function)", "approverTypeIsOrgScoped": "src/automation/approval.zod.ts#approverTypeIsOrgScoped (function)", + "builtinNodeConfigKeysJudged": "src/automation/flow-node-config-refusals.ts#builtinNodeConfigKeysJudged (function)", "canonicalApproverType": "src/automation/approval.zod.ts#canonicalApproverType (function)", "collectFlowGraphs": "src/automation/control-flow.zod.ts#collectFlowGraphs (function)", "defineActionDescriptor": "src/automation/node-executor.zod.ts#defineActionDescriptor (function)", From 72b8d3e978667f440dbb75d6c5a282f2a1accd5d Mon Sep 17 00:00:00 2001 From: Claude Date: Thu, 8 Oct 2026 15:48:45 +0000 Subject: [PATCH 07/10] fix(metadata-protocol): the save door's flow fallback judges the D2-converted body, stores the raw one; re-point two spec controls (wip) Claude-Session: https://claude.ai/code/session_01DhTqaEHqPVSVnAkjG3jywn Co-authored-by: Claude --- ...tin-node-config-undeclared-keys-refused.md | 5 +- ...rotocol.save-flow-canonicalization.test.ts | 57 +++++++++++++++++++ packages/metadata-protocol/src/protocol.ts | 26 ++++++++- .../flow-builtin-node-config-values.test.ts | 11 +++- .../flow-node-config-required.test.ts | 8 ++- 5 files changed, 101 insertions(+), 6 deletions(-) diff --git a/.changeset/21982-flow-builtin-node-config-undeclared-keys-refused.md b/.changeset/21982-flow-builtin-node-config-undeclared-keys-refused.md index 2e8e4850fe4..9ea7f58ccde 100644 --- a/.changeset/21982-flow-builtin-node-config-undeclared-keys-refused.md +++ b/.changeset/21982-flow-builtin-node-config-undeclared-keys-refused.md @@ -1,11 +1,12 @@ --- '@objectstack/spec': minor '@objectstack/service-automation': patch +'@objectstack/metadata-protocol': minor --- A key a builtin flow node's executor contract does not declare is refused at parse, with a location, on every builtin but `try_catch`: a `notify` `bogusKey`, a screen field's `visibleIf`, a `create_record` `fieldValues`, an `http` `outputVariable` and the like no longer pass `objectstack validate` and `objectstack compile` and then get the whole flow refused at registration. -Clause-②: no (narrowing) +Clause-②: yes (narrowing: an undeclared config key on 10 more builtins, every strict-contract builtin but try_catch, is refused at the build doors and the save door, where it passed; widening: a save door with no flow canonicalizer judges the D2-converted body, so a D2 alias spelling it refuses today, script functionName / input and subflow flow, is accepted again, as at every other door) @@ -17,6 +18,8 @@ Clause-②: no (narrowing) **`@objectstack/service-automation`.** `registerFlow`'s descriptor walk (`validateNodeConfigKeys`) stands aside for every type `builtinNodeConfigKeysJudged` names, so each node type has one judge. Before the move, the declared key sets were measured equal: on each of these types the descriptor's declared keys, at every position the walk descends to, equal the keys the contract accepts there. So registration refuses exactly what it refused before. The refusal now arrives as the parse's located issue instead of the walk's `Flow '…' rejected: N undeclared config key(s)` text. +**`@objectstack/metadata-protocol`.** The save door judges a flow in its canonical spelling on every path. When no flow canonicalizer resolved (a host with no automation service), or the canonicalizer threw (a draft with a temporary cycle), the schema gate now judges the body with the ADR-0087 D2 conversions applied (`applyConversionsToFlow`), for the verdict only. The stored body stays the raw request body, exactly as before. A D2 spelling the load path still rewrites (`filters` on a CRUD node; `to`, `subject`, `body` and `url` on a `notify`; `flow` on a `map`; `functionName` and `input` on a `script`; `flow` on a `subflow`) is therefore accepted at that door, as at every other one, and a key no conversion rewrites is refused there, located. This widens the fallback path back for the `script` and `subflow` aliases the previous key arm had begun refusing there. `duplicatePackage` re-saves through the same gate. + **What stays as it was.** - `try_catch`: its undeclared keys stay registration's, judged by the walk. Its contract's `retry` is the shared `RetryPolicySchema`, which strips an unknown key, while its descriptor closes `retry` to `maxRetries`, `backoffMs`, `backoffMultiplier`, `maxRetryDelayMs` and `jitter`. Moving the judge would have widened registration. diff --git a/packages/metadata-protocol/src/protocol.save-flow-canonicalization.test.ts b/packages/metadata-protocol/src/protocol.save-flow-canonicalization.test.ts index 183ed686786..1195c6d48a2 100644 --- a/packages/metadata-protocol/src/protocol.save-flow-canonicalization.test.ts +++ b/packages/metadata-protocol/src/protocol.save-flow-canonicalization.test.ts @@ -397,6 +397,63 @@ describe('saveMetaItem canonicalizes flow bodies (#4542)', () => { expect(fallbackWarnings()).toHaveLength(0); }); + /** + * [#21982] On the fallback path — no canonicalizer resolved, or it threw — + * the gate judges the body in its ADR-0087 D2-converted spelling, for the + * verdict only. The flow parse refuses an undeclared config key on the + * builtin node types, so without the conversion a D2 spelling every other + * door converts first (`os validate`, `defineStack`, `registerFlow`) would + * be refused here, and only here: a verdict that depended on whether the + * host runs an automation service. What is STORED stays the raw body. + */ + describe('[#21982] the fallback gate judges the D2-converted body, and stores the raw one', () => { + const scriptBody = (config: Record) => ({ + ...flowBody({}), + nodes: [{ id: 'n1', type: 'script', label: 'Triage', config }], + }); + const issuesOf = async (promise: Promise) => { + try { + await promise; + } catch (err) { + return err as { code?: unknown; status?: unknown; issues?: Array<{ path: string }> }; + } + throw new Error('expected the save to be refused'); + }; + + it('a legacy script `functionName` body saves on a host with no automation service, stored raw', async () => { + const { protocol, rows } = makeProtocol(new Map()); + + const result = await save(protocol, scriptBody({ functionName: 'helpdesk.triage' })); + + expect(result.success).toBe(true); + // Judged as `function`, stored exactly as written. + expect(storedFlow(rows)!.body.nodes[0].config).toEqual({ functionName: 'helpdesk.triage' }); + }); + + it('an undeclared key on delete_record is refused 422 INVALID_METADATA at its path — the alias beside it is not', async () => { + const { protocol, rows } = makeProtocol(new Map()); + + const err = await issuesOf(save(protocol, flowBody({ objectName: 'lead', filters: { status: 'stale' }, bogusKey: 1 }))); + + expect({ code: err.code, status: err.status }).toEqual({ code: 'INVALID_METADATA', status: 422 }); + expect((err.issues ?? []).map((i) => i.path)).toEqual(['nodes.0.config.bogusKey']); + expect(rows.size).toBe(0); + }); + + it('a canonicalizer throw still refuses a body the gate rejects after conversion', async () => { + const throwing = () => { throw new Error('cycle detected: n1 → n1'); }; + const { protocol, rows } = makeProtocol( + new Map([['automation', { canonicalizeStoredFlow: throwing }]]), + ); + + const err = await issuesOf(save(protocol, flowBody({ objectName: 'lead', filters: { status: 'stale' }, bogusKey: 1 }))); + + expect({ code: err.code, status: err.status }).toEqual({ code: 'INVALID_METADATA', status: 422 }); + expect((err.issues ?? []).map((i) => i.path)).toEqual(['nodes.0.config.bogusKey']); + expect(rows.size).toBe(0); + }); + }); + it('non-flow saves never consult the canonicalizer', async () => { const spy = vi.fn(canonicalizeStoredFlow); const { protocol } = makeProtocol( diff --git a/packages/metadata-protocol/src/protocol.ts b/packages/metadata-protocol/src/protocol.ts index 3c2f7d11afc..59d2c70179a 100644 --- a/packages/metadata-protocol/src/protocol.ts +++ b/packages/metadata-protocol/src/protocol.ts @@ -150,7 +150,7 @@ import { PLURAL_TO_SINGULAR, SINGULAR_TO_PLURAL, canonicalMetaUrlType, metaUrlSp // depends on `@objectstack/service-cluster`; a bridge plugin there hands the // live transport in through `attachMetadataMutationPubSub`. import type { IObjectQLEngine, IPubSub, ISecurityService } from '@objectstack/spec/contracts'; -import { applyConversionsToStoredItem, type ConversionNotice, type ConversionTodoNotice } from '@objectstack/spec'; +import { applyConversionsToFlow, applyConversionsToStoredItem, type ConversionNotice, type ConversionTodoNotice } from '@objectstack/spec'; import { type FormView, type I18nLabel, isAggregatedViewContainer, expandViewContainer, resolveI18nLabel } from '@objectstack/spec/ui'; // [commit ece4dad31] Emitted-specifier pin. This module's inferred public declarations // structurally mention `FormFieldInput` (FormView `sections[].fields`), and @@ -20550,11 +20550,28 @@ export class ObjectStackProtocolImplementation implements // an already-canonical body comes back reference-identical, so // `migrateStoredMetadata` and `duplicatePackage` re-entering here pay // nothing. + // + // [#21982] The schema gate below JUDGES a flow in its canonical + // spelling on every path. When no canonicalizer resolved, or it threw, + // `flowGateVerdictBody` is the raw body with the spec's ADR-0087 D2 + // conversions applied (`applyConversionsToFlow`) — for the VERDICT + // only: what is stored stays the raw request body, exactly as before. + // The flow parse judges an undeclared config key on the builtin node + // types, so a D2 spelling the load path still rewrites (`filters`, + // script `functionName`, subflow `flow`, …) would otherwise be refused + // here and accepted at every converting door (`os validate`, + // `defineStack`, `registerFlow`): a verdict that depended on whether + // this host runs an automation service. No `reservedNodeTypes` (no + // engine here): a node-type rename's conflict guard keeps its one home + // in the engine's canonicalizer, and this converted body is never + // persisted. A key no conversion rewrites is judged exactly as written. + let flowGateVerdictBody: unknown; if (singularType === 'flow' && request.item) { // No automation service reachable (control-plane / metadata-only // host): save exactly as today — a host must not start refusing // flow writes it accepted yesterday. const canonicalizeFlow = this.resolveFlowCanonicalizer(); + let canonicalized = false; if (canonicalizeFlow) { let result: StoredFlowCanonicalization | undefined; try { @@ -20616,8 +20633,10 @@ export class ObjectStackProtocolImplementation implements throw err; } request.item = result.storable; + canonicalized = true; } } + if (!canonicalized) flowGateVerdictBody = applyConversionsToFlow(request.item); } // Spec-conformance check: if a Zod schema is registered for this @@ -20642,7 +20661,10 @@ export class ObjectStackProtocolImplementation implements { const schema = resolveOverlaySchema(request.type, request.item); if (schema) { - const parsed = schema.safeParse(request.item); + // [#21982] A flow on the canonicalizer's fallback is judged in + // its D2-converted spelling (see `flowGateVerdictBody` above); + // every other body is judged as stored. + const parsed = schema.safeParse(flowGateVerdictBody ?? request.item); if (!parsed.success) { const issues = zodIssuesToMetadataIssues(parsed.error.issues); // [#10524 → commit d806081dd] The findings clause is rendered PER diff --git a/packages/spec/src/automation/flow-builtin-node-config-values.test.ts b/packages/spec/src/automation/flow-builtin-node-config-values.test.ts index 9cedf79087d..93324d826ea 100644 --- a/packages/spec/src/automation/flow-builtin-node-config-values.test.ts +++ b/packages/spec/src/automation/flow-builtin-node-config-values.test.ts @@ -219,8 +219,15 @@ describe('what stays accepted (lit controls)', () => { }); it('CONTROL: key membership is not this arm\'s — an undeclared key and a tombstoned one', () => { - expect(flowNodeConfigRefusals('http', { ...VALID.http, bogusKey: 1 })).toEqual([]); - expect(flowNodeConfigRefusals('screen', { fields: [{ name: 'qty', visible: 'x' }] })).toEqual([]); + // [#21982] An undeclared key on these builtins draws the KEY arm's one + // refusal at its location; this arm adds nothing beside it. + for (const [type, config, key] of [ + ['http', { ...VALID.http, bogusKey: 1 }, 'bogusKey'], + ['screen', { fields: [{ name: 'qty', visible: 'x' }] }, 'fields[0].visible'], + ] as const) { + expect(flowNodeConfigRefusals(type, config).map(({ code, path }) => ({ code, path })), type) + .toEqual([{ code: 'node-config-refused-by-contract', path: key }]); + } expect(flowNodeConfigRefusals('script', { ...VALID.script, actionType: 'email' })).toEqual([]); expect(flowNodeConfigRefusals('try_catch', { ...VALID.try_catch, retry: { retryDelayMs: 500 } })).toEqual([]); // CONTROL: the contracts refuse all four — the arm holds back, the contract does not. 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 9322ab3e43b..03f525120ea 100644 --- a/packages/spec/src/automation/flow-node-config-required.test.ts +++ b/packages/spec/src/automation/flow-node-config-required.test.ts @@ -180,7 +180,13 @@ describe('FlowSchema.parse refuses a key the node\'s executor contract requires, }); 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([]); + // [#21982] An undeclared key is the KEY arm's: its one refusal, at the key — + // never a `node-config-key-missing` or rule finding from this rule. + const undeclared = { url: 'https://example.com', zzz_undeclared: 1 }; + expect(issuesOf(flowWith(node('http', undeclared))).map((i) => ({ code: i.code, path: i.path.join('.') }))) + .toEqual([{ code: 'custom', path: 'nodes.1.config.zzz_undeclared' }]); + expect(flowNodeConfigRefusals('http', undeclared).map(({ code, path }) => ({ code, path }))) + .toEqual([{ code: 'node-config-refused-by-contract', path: 'zzz_undeclared' }]); }); it('a node type with no executor contract is untouched — `assignment`, a plugin type', () => { From d7466a01bca61c28c401c4bc30047f96680cddf8 Mon Sep 17 00:00:00 2001 From: Claude Date: Thu, 8 Oct 2026 16:02:18 +0000 Subject: [PATCH 08/10] fix(metadata-protocol): the runtime authoring gate judges the same D2-converted verdict body on the flow fallback (wip) Claude-Session: https://claude.ai/code/session_01DhTqaEHqPVSVnAkjG3jywn Co-authored-by: Claude --- packages/metadata-protocol/src/protocol.ts | 11 ++++++++--- 1 file changed, 8 insertions(+), 3 deletions(-) diff --git a/packages/metadata-protocol/src/protocol.ts b/packages/metadata-protocol/src/protocol.ts index 59d2c70179a..f15e4802b52 100644 --- a/packages/metadata-protocol/src/protocol.ts +++ b/packages/metadata-protocol/src/protocol.ts @@ -20551,8 +20551,8 @@ export class ObjectStackProtocolImplementation implements // `migrateStoredMetadata` and `duplicatePackage` re-entering here pay // nothing. // - // [#21982] The schema gate below JUDGES a flow in its canonical - // spelling on every path. When no canonicalizer resolved, or it threw, + // [#21982] The two gates below — the schema gate and the runtime + // authoring gate — JUDGE a flow in its canonical spelling on every path. When no canonicalizer resolved, or it threw, // `flowGateVerdictBody` is the raw body with the spec's ADR-0087 D2 // conversions applied (`applyConversionsToFlow`) — for the VERDICT // only: what is stored stays the raw request body, exactly as before. @@ -20777,7 +20777,12 @@ export class ObjectStackProtocolImplementation implements type: request.type, name: request.name, state: mode === 'draft' ? 'draft' : 'active', - body: gatedItem, + // [#21982] The same verdict body as the schema gate above: a flow + // on the canonicalizer's fallback is judged in its D2-converted + // spelling (the lint's config judge refuses an undeclared key, so + // the raw `filters` alias would be refused here and nowhere else). + // Stored, and handed to the credential walk below, as written. + body: flowGateVerdictBody ?? gatedItem, source: writeSource, // [#6285] The write's organization partition. It was always here; // it simply never travelled to the gate, which is the whole reason From beda06e97812716e44a07ab84688df64e161fdc4 Mon Sep 17 00:00:00 2001 From: Claude Date: Thu, 8 Oct 2026 16:55:29 +0000 Subject: [PATCH 09/10] docs(changeset): the metadata-protocol paragraph names both gates and refuses only an undeclared key Claude-Session: https://claude.ai/code/session_01DhTqaEHqPVSVnAkjG3jywn Co-authored-by: Claude --- .../21982-flow-builtin-node-config-undeclared-keys-refused.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.changeset/21982-flow-builtin-node-config-undeclared-keys-refused.md b/.changeset/21982-flow-builtin-node-config-undeclared-keys-refused.md index 9ea7f58ccde..36003ff0e79 100644 --- a/.changeset/21982-flow-builtin-node-config-undeclared-keys-refused.md +++ b/.changeset/21982-flow-builtin-node-config-undeclared-keys-refused.md @@ -18,7 +18,7 @@ Clause-②: yes (narrowing: an undeclared config key on 10 more builtins, every **`@objectstack/service-automation`.** `registerFlow`'s descriptor walk (`validateNodeConfigKeys`) stands aside for every type `builtinNodeConfigKeysJudged` names, so each node type has one judge. Before the move, the declared key sets were measured equal: on each of these types the descriptor's declared keys, at every position the walk descends to, equal the keys the contract accepts there. So registration refuses exactly what it refused before. The refusal now arrives as the parse's located issue instead of the walk's `Flow '…' rejected: N undeclared config key(s)` text. -**`@objectstack/metadata-protocol`.** The save door judges a flow in its canonical spelling on every path. When no flow canonicalizer resolved (a host with no automation service), or the canonicalizer threw (a draft with a temporary cycle), the schema gate now judges the body with the ADR-0087 D2 conversions applied (`applyConversionsToFlow`), for the verdict only. The stored body stays the raw request body, exactly as before. A D2 spelling the load path still rewrites (`filters` on a CRUD node; `to`, `subject`, `body` and `url` on a `notify`; `flow` on a `map`; `functionName` and `input` on a `script`; `flow` on a `subflow`) is therefore accepted at that door, as at every other one, and a key no conversion rewrites is refused there, located. This widens the fallback path back for the `script` and `subflow` aliases the previous key arm had begun refusing there. `duplicatePackage` re-saves through the same gate. +**`@objectstack/metadata-protocol`.** The save door judges a flow in its canonical spelling on every path. When no flow canonicalizer resolved (a host with no automation service), or the canonicalizer threw (a draft with a temporary cycle), the schema gate and the runtime authoring gate now judge the body with the ADR-0087 D2 conversions applied (`applyConversionsToFlow`), for the verdict only. The stored body stays the raw request body, exactly as before. A D2 spelling the load path still rewrites (`filters` on a CRUD node; `to`, `subject`, `body` and `url` on a `notify`; `flow` on a `map`; `functionName` and `input` on a `script`; `flow` on a `subflow`) is therefore accepted at that door, as at every other one, and an undeclared key that no conversion rewrites is refused there, located. This widens the fallback path back for the `script` and `subflow` aliases the previous key arm had begun refusing there. `duplicatePackage` re-saves through the same gate. **What stays as it was.** From 6e56f2f68a5439d2ed797a91e7016750782a9662 Mon Sep 17 00:00:00 2001 From: Claude Date: Thu, 8 Oct 2026 17:33:45 +0000 Subject: [PATCH 10/10] docs(automation): a builtin's undeclared config key is refused at the flow parse; try_catch and plugin types stay on registerFlow's walk Claude-Session: https://claude.ai/code/session_01DhTqaEHqPVSVnAkjG3jywn Co-authored-by: Claude --- content/docs/automation/flows.mdx | 24 +++++++++++++++++------- 1 file changed, 17 insertions(+), 7 deletions(-) diff --git a/content/docs/automation/flows.mdx b/content/docs/automation/flows.mdx index 23ec0599a84..0c4a672555c 100644 --- a/content/docs/automation/flows.mdx +++ b/content/docs/automation/flows.mdx @@ -138,7 +138,7 @@ Each node performs a specific action in the flow. | `id` | `string` | ✅ | Unique node identifier — unique across the **whole flow**: the top-level `nodes[]` and every region body (`loop.body`, `parallel.branches[]`, `try_catch.try` / `.catch`, at any depth) share one id space, and `FlowSchema` refuses a reused id at parse (`Duplicate node id …`, naming both locations) | | `type` | `string` | ✅ | Node type — a built-in id from the table above **or** a plugin-registered one. Per ADR-0018 the spec does not gate this with a closed enum; it is checked against the live action registry once that registry is complete — plugins contribute node types while they start, so flows registered during boot are checked in one pass when the vocabulary closes (all plugins started), and anything registered after that (Studio publish, dev reload) is checked immediately. Unknown types warn, never reject; executing one fails with `NO_EXECUTOR` | | `label` | `string` | ✅ | Display label | -| `config` | `object` | optional | Type-specific configuration — the registered executor's `configSchema` owns its shape. Keys that schema does not declare are rejected at `registerFlow()`, an `approval` node's config is judged whole against its declared contract at the flow parse, and the built-in executors `parse()` the value against their Zod contract before running (#4277) | +| `config` | `object` | optional | Type-specific configuration — its node type owns its shape. On a built-in node type with an executor contract other than `try_catch`, a key that contract does not declare is refused at the flow parse, at the key's path; on `try_catch` and on a plugin node type, a key the published `configSchema` does not declare is rejected at `registerFlow()`. An `approval` node's config is judged whole against its declared contract at the flow parse, and the built-in executors `parse()` the value against their Zod contract before running | | `connectorConfig` | `object` | ✅ on `connector_action` | `{ connectorId, actionId, input }` — the only input a `connector_action` node's executor reads. `FlowSchema` refuses a `connector_action` node without it, and one whose `connectorId` or `actionId` is blank (empty or whitespace only), at any depth including a region body. `connectorId` is the registered connector's `name`, `actionId` one of the action keys it declares; `input` is optional | | `position` | `{ x, y }` | optional | Visual position on canvas | | `timeoutMs` | `number` | optional | Per-node execution timeout | @@ -149,13 +149,23 @@ Each node performs a specific action in the flow. The flow, node, edge, and variable **shells are `.strict()`** — a key they do not declare is a parse error naming the likely intended key, not a silent strip. A -node's `config` is open at the flow-parse level (plugin node types keep their own -vocabulary), but it is not unchecked: `registerFlow()` rejects config keys the -node type's published `configSchema` does not declare — naming the path, the -declared key set, and a did-you-mean — and the contract-carrying builtins +node's `config` has no single shape at the flow-parse level (each node type owns +its vocabulary), but it is not unchecked. On every built-in node type with an +executor contract except `try_catch` — `get_record`, `create_record`, +`update_record`, `delete_record`, `notify`, `http`, `screen`, `map`, `loop`, +`parallel`, `script` and `subflow` — the flow parse refuses a config key that +contract does not declare, at the key's own path and in the contract's words +(its did-you-mean, or its prescription for a known slip such as `fieldValues` → +`fields`), so `objectstack validate`, `objectstack compile`, the metadata save +door and `registerFlow()` all refuse it before anything runs (a key on a `loop` +body or a `parallel` branch, or on a region's nodes and edges, is the region +check's at `registerFlow()`). A `try_catch` node +and a plugin node type are judged at `registerFlow()` instead, against the +`configSchema` the node type publishes — naming the path, the declared key set, +and a did-you-mean; a plugin node type that publishes no `configSchema` declares +nothing, so nothing can be undeclared. The contract-carrying builtins additionally `parse()` their config at execute time, refusing the node on a type -or missing-`required` violation (#4277). A node type that publishes no -`configSchema` declares nothing, so nothing can be undeclared. An `approval` +or missing-`required` violation. An `approval` node's config is judged whole by the flow parse, against the contract the spec declares for it: an undeclared key or a value its executor would refuse (`escalation.timeoutHours: 0.5` under its `>= 1`) refuses the flow at the