From f56deadf6eae394a9befe517834d4514db7d0749 Mon Sep 17 00:00:00 2001 From: Claude Date: Thu, 15 Oct 2026 22:21:01 +0000 Subject: [PATCH 1/4] feat(spec)!: refuse the wait / connector_action loose config keys; their D2 lifts leave the load path (F3) Co-authored-by: Claude Claude-Session: https://claude.ai/code/session_011ZyGVR3TdcTDLhuzyEbkBN --- .../flow-sibling-block-loose-keys.ts | 98 +++++++++ packages/spec/src/automation/flow.zod.ts | 20 +- packages/spec/src/conversions/registry.ts | 190 +++++++++++------- 3 files changed, 235 insertions(+), 73 deletions(-) create mode 100644 packages/spec/src/automation/flow-sibling-block-loose-keys.ts diff --git a/packages/spec/src/automation/flow-sibling-block-loose-keys.ts b/packages/spec/src/automation/flow-sibling-block-loose-keys.ts new file mode 100644 index 00000000000..8bc10e040ce --- /dev/null +++ b/packages/spec/src/automation/flow-sibling-block-loose-keys.ts @@ -0,0 +1,98 @@ +// Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license. + +/** + * @module automation/flow-sibling-block-loose-keys + * + * **The loose `config` keys of the two node types whose contract is a SIBLING + * block** — `wait` (`waitEventConfig`) and `connector_action` + * (`connectorConfig`) — and the one refusal an author meets for writing one + * (#23543 F3). + * + * Neither node type has a `config` contract: each executor reads its sibling + * block and nothing else (`service-automation`'s `wait-node.ts`, + * `connector-nodes.ts`). Through 17 the ADR-0087 D2 conversions + * `flow-node-wait-event-config-lift` and `flow-node-connector-config-lift` + * lifted these keys from `config` into the block at load. At 18 both left the + * load path, so an authored loose key would be read by nobody — a loose `input` + * beside a complete `connectorConfig` would dispatch the action with no input. + * The flow parse refuses each one instead, naming the block key it belongs on. + * + * ONE table, two readers, so they cannot drift: the refusal below, and the two + * lifts in `conversions/registry.ts`, which still run at every stored-row + * rehydration seam (`includeRetired`) and delete a loose key the block already + * shadows, so a stored flow the refusal would reject keeps loading. The keys are + * exactly the ones the lifts move — ⛔ never a key outside them: an unknown + * `config` key on these node types was never lifted and is not this refusal's. + * + * Not exported from the automation barrel: it is a leaf both readers import. + */ + +/** + * The loose `config` keys the `wait` executor used to also accept, in the exact + * precedence order of the `??` chains it carried, keyed by the declared + * `waitEventConfig` property each one feeds. + * + * `duration` and `signal` are not declared names anywhere in the spec — they + * only ever existed as the tail of an executor fallback, which is why the + * declared spelling is listed first in each group. + */ +export const WAIT_EVENT_CONFIG_LOOSE_KEYS: ReadonlyArray = [ + ['eventType', ['eventType']], + ['timerDuration', ['timerDuration', 'duration']], + ['signalName', ['signalName', 'signal']], + ['timeoutMs', ['timeoutMs']], +]; + +/** The `config` keys a mis-taught `connector_action` node carries, in declared-block order. */ +export const CONNECTOR_CONFIG_LOOSE_KEYS = ['connectorId', 'actionId', 'input'] as const; + +/** The house sentence for an edit `os migrate meta --from 16` lists (both lifts are step-17 members). */ +const MIGRATE_META_SENTENCE = + 'Run `os migrate meta --from 16` to list the mechanical edits for existing sources; `--write` applies the ones it can prove, and you apply the rest by hand.'; + +/** A plain object (not an array, not `null`). */ +function isRecord(value: unknown): value is Record { + return typeof value === 'object' && value !== null && !Array.isArray(value); +} + +/** `loose key → the block key it belongs on`, per node type. */ +function looseKeyTargets(type: unknown): ReadonlyMap | undefined { + if (type === 'wait') { + return new Map(WAIT_EVENT_CONFIG_LOOSE_KEYS.flatMap(([target, candidates]) => candidates.map((c) => [c, target] as const))); + } + if (type === 'connector_action') return new Map(CONNECTOR_CONFIG_LOOSE_KEYS.map((k) => [k, k] as const)); + return undefined; +} + +/** + * Every named loose key under a `wait` / `connector_action` node's `config`, + * each refused at `config.` with the prescription naming the sibling block + * key it belongs on. Judged by the `FlowSchema` superRefine for every node the + * `collectFlowGraphs` walk reaches, beside `connectorActionConfigRefusals`, so a + * node in an ADR-0031 region body is refused where the author wrote it. + * + * A key is refused when it is PRESENT, whatever its value: the block is the only + * place the executor reads, so a loose key carries nothing either way. + */ +export function siblingBlockLooseConfigRefusals(node: unknown): Array<{ path: PropertyKey[]; message: string }> { + if (!isRecord(node)) return []; + const targets = looseKeyTargets(node.type); + if (targets === undefined || !isRecord(node.config)) return []; + const block = node.type === 'wait' ? 'waitEventConfig' : 'connectorConfig'; + const out: Array<{ path: PropertyKey[]; message: string }> = []; + for (const key of Object.keys(node.config)) { + const target = targets.get(key); + if (target === undefined || node.config[key] === undefined) continue; + const rename = target === key ? '' : ` (the key is \`${target}\` there; \`${key}\` was only ever an executor fallback)`; + out.push({ + path: ['config', key], + message: + `A \`${String(node.type)}\` node's \`config.${key}\` was removed in @objectstack/spec 18 (ADR-0087 D2) — this ` + + `node type has no \`config\` contract: its executor reads only the \`${block}\` block, and the loader lifted ` + + `this key into it until this release. Move it to \`${block}.${target}\`${rename}; the value is unchanged. If ` + + `\`${block}.${target}\` is already set, it is the one the node runs with — delete \`config.${key}\`. ` + + MIGRATE_META_SENTENCE, + }); + } + return out; +} diff --git a/packages/spec/src/automation/flow.zod.ts b/packages/spec/src/automation/flow.zod.ts index f4c8a816f55..f69316ecce7 100644 --- a/packages/spec/src/automation/flow.zod.ts +++ b/packages/spec/src/automation/flow.zod.ts @@ -26,6 +26,9 @@ import { strictObject } from '../shared/strict-object'; import { collectFlowGraphs, parseFlowNodeRegions } from './control-flow.zod'; import { predicateSlotRefusal, resolveFlowNodeExpressions } from './flow-node-expression-paths'; import { flowNodeConfigRefusals } from './flow-node-config-refusals'; +// [#23543 F3] The loose `config` keys of the two sibling-block node types — one +// table, shared with the two D2 lifts that still run at rehydration. +import { siblingBlockLooseConfigRefusals } from './flow-sibling-block-loose-keys'; import { EndConfigSchema } from './builtin-node-config.zod'; // [#22572] The `$` names are the flow engine's at every binding door — the one // rule, composed into a declared variable's `name` and judging an `assignment` @@ -455,7 +458,9 @@ const CONNECTOR_DISPATCH_KEYS = ['connectorId', 'actionId'] as const; * * - **the block absent** — `{ type: 'connector_action' }`, the shape of a node * authored without its configuration, or with the keys left under `config` - * where the load-time conversion cannot complete the pair; + * (since 18 each such key is refused on its own as well, + * `siblingBlockLooseConfigRefusals`; a stored row's lift completes the pair + * when it can); * - **an id present and blank** — `connectorConfig: { connectorId: '', * actionId: '' }` is the Studio designer's seed for a freshly added node, so * a node added and saved before it is configured is this shape. @@ -1583,6 +1588,19 @@ export const FlowSchema = lazySchema(() => strictObject( }); } + // [#23543 F3] A `wait` / `connector_action` node's settings live in its + // sibling block only, and the loader no longer lifts the named loose + // `config` keys into it (both D2 lifts left the load path at 18). Each such + // key is refused where it is written, naming the block key it belongs on — + // in this walk, like the rule above, so a region-body node is reached too. + for (const graph of collectFlowGraphs(flow)) { + graph.nodes.forEach((node, index) => { + for (const refusal of siblingBlockLooseConfigRefusals(node)) { + ctx.addIssue({ code: 'custom', path: [...graph.path, 'nodes', index, ...refusal.path], message: refusal.message }); + } + }); + } + // Edges (#14964): every reader of `edges[].id` assumes the ids are unique — // a designer, a BPMN export, a flow diff, any traversal that dedupes by id — // while nothing enforced it: two edges carrying one id parsed, shipped diff --git a/packages/spec/src/conversions/registry.ts b/packages/spec/src/conversions/registry.ts index 3e1bfb1b04f..940997a39a4 100644 --- a/packages/spec/src/conversions/registry.ts +++ b/packages/spec/src/conversions/registry.ts @@ -46,6 +46,7 @@ import { } from '../ui/view.zod.js'; import { ASSEMBLED_VIEW_ITEMS_KEY } from '../ui/assembled-views.zod.js'; import { FLOW_REGION_SLOTS_BY_TYPE } from '../automation/region-slots.js'; +import { CONNECTOR_CONFIG_LOOSE_KEYS, WAIT_EVENT_CONFIG_LOOSE_KEYS } from '../automation/flow-sibling-block-loose-keys.js'; /** * Flow callout node type rename (protocol 11.0). @@ -1277,18 +1278,12 @@ const flowNodeNotifyConfigAliases: MetadataConversion = { /** * The loose `config` keys the `wait` executor used to also accept, in the exact * precedence order of the `??` chains it carried, keyed by the declared - * `waitEventConfig` property each one feeds. - * - * `duration` and `signal` are not declared names anywhere in the spec — they - * only ever existed as the tail of an executor fallback, which is why the - * declared spelling is listed first in each group. + * `waitEventConfig` property each one feeds. Declared once in + * `automation/flow-sibling-block-loose-keys.ts`, the table the flow parse's + * loose-key refusal reads too (#23543 F3), so the keys refused at authoring and + * the keys lifted at rehydration are one set. */ -const WAIT_EVENT_CONFIG_LIFTS: ReadonlyArray = [ - ['eventType', ['eventType']], - ['timerDuration', ['timerDuration', 'duration']], - ['signalName', ['signalName', 'signal']], - ['timeoutMs', ['timeoutMs']], -]; +const WAIT_EVENT_CONFIG_LIFTS = WAIT_EVENT_CONFIG_LOOSE_KEYS; /** * Lift `wait`'s loose `config.*` event keys onto the declared `waitEventConfig` @@ -1312,16 +1307,20 @@ const WAIT_EVENT_CONFIG_LIFTS: ReadonlyArray { @@ -1618,40 +1646,58 @@ function liftConnectorConfigShape(stack: Dict, emit: Emit): Dict { const cc: Dict = isDict(node.connectorConfig) ? { ...node.connectorConfig } : {}; const nextConfig: Dict = { ...config }; const lifts: Array<(typeof CONNECTOR_CONFIG_LIFTS)[number]> = []; + const drops: Array<{ key: (typeof CONNECTOR_CONFIG_LIFTS)[number]; empty: boolean }> = []; for (const key of CONNECTOR_CONFIG_LIFTS) { - if (nextConfig[key] == null) continue; - if (cc[key] != null) continue; // declared block wins; the loose key stays shadowed - cc[key] = nextConfig[key]; + if (!(key in nextConfig)) continue; + const value = nextConfig[key]; delete nextConfig[key]; - lifts.push(key); + if (value != null && cc[key] == null) { + cc[key] = value; + lifts.push(key); + } else { + drops.push({ key, empty: value == null }); // declared block wins, or nothing to carry + } } - if (lifts.length === 0) return node; + if (lifts.length === 0 && drops.length === 0) return node; // Completeness guard — never materialize a block the loader would reject. if (cc.connectorId == null || cc.actionId == null) return node; for (const key of lifts) { emit({ from: `config.${key}`, to: `connectorConfig.${key}`, path: `${path}.connectorConfig.${key}` }); } - return { ...node, config: nextConfig, connectorConfig: cc }; + for (const { key, empty } of drops) { + const by = empty ? 'it carried no value' : `\`connectorConfig.${key}\` already set, so it was never read`; + emit({ from: `config.${key}`, to: `(removed — ${by})`, path: `${path}.config` }); + } + return lifts.length > 0 ? { ...node, config: nextConfig, connectorConfig: cc } : { ...node, config: nextConfig }; }); } /** * Connector flow-node loose `config` keys → the declared `connectorConfig` - * sibling (protocol 17, #4045). See {@link liftConnectorConfigShape} for the - * precedence rules and the completeness guard. - * - * **Stays on the load path at 18 — a recorded verdict (#23487).** Like `wait`, - * `connector_action` has no `config` contract, so retired, a loose key beside - * a complete `connectorConfig` would be silently ignored: a loose `input` next - * to a declared pair would dispatch the action with no input. Refusing the - * loose keys under `config` would also refuse stored flows whose loose key the - * block shadows (the rehydration seam parses the same schema), so it can retire - * once that shadowed shape has a ruling. + * sibling (protocol 17, #4045; retired from the load path at 18, #23543 F3). + * See {@link liftConnectorConfigShape} for the precedence rules, the + * completeness guard, and why a shadowed loose key is deleted. + * + * ## Retired from the load path at 18 (#23543, ruling F3 A) + * + * Through 17 it stayed live because, like `wait`, `connector_action` has no + * `config` contract, so retired alone, a loose key beside a complete + * `connectorConfig` would have been silently ignored: a loose `input` next to a + * declared pair would dispatch the action with no input (the verdict #23487 + * recorded). The ruling closed both halves: the flow parse refuses + * `config.connectorId` / `config.actionId` / `config.input` on a + * `connector_action` node with the prescription naming `connectorConfig` + * (`siblingBlockLooseConfigRefusals`), and this lift deletes the loose keys the + * block shadows, so a stored flow the refusal would reject keeps loading. The + * seams that still replay it are the ones listed on + * `flow-node-wait-event-config-lift`. */ const flowNodeConnectorConfigLift: MetadataConversion = { id: 'flow-node-connector-config-lift', toMajor: 17, + retiredFromLoadPath: true, + retiredAfter: '17.7.0', surface: 'flow.node.connector_action.connectorConfig', summary: "connector_action flow-node loose config keys 'connectorId' / 'actionId' / 'input' → " + @@ -1677,8 +1723,8 @@ const flowNodeConnectorConfigLift: MetadataConversion = { input: { channel: 'C0WINS000', text: 'Task done: {record.title}' }, }, }, - // A declared block WINS: `config.connectorId` stays shadowed in - // place, and since nothing else lifts, the node is untouched. + // A declared block WINS: the shadowed `config.connectorId` is + // deleted (#23543 F3) and the block is untouched. { id: 'n3', type: 'connector_action', @@ -1725,7 +1771,7 @@ const flowNodeConnectorConfigLift: MetadataConversion = { id: 'n3', type: 'connector_action', connectorConfig: { connectorId: 'rest', actionId: 'get', input: {} }, - config: { connectorId: 'ignored' }, + config: {}, }, { id: 'n4', @@ -1742,9 +1788,9 @@ const flowNodeConnectorConfigLift: MetadataConversion = { }, ], }, - // n2: the full trio lifts. n3: shadowed → no lift, no notice. n4: guard — + // n2: the full trio lifts. n3: shadowed → deleted, one notice. n4: guard — // nothing materializes. n5: the single unshadowed `input`. - expectedNotices: 4, + expectedNotices: 5, }, }; From 3abeb68e971504a4a35afcbf850a83449af5d32a Mon Sep 17 00:00:00 2001 From: Claude Date: Thu, 15 Oct 2026 22:25:28 +0000 Subject: [PATCH 2/4] test(spec,service-automation): pin the F3 refusal, the stored-row lifts and the shadowed-key delete Co-authored-by: Claude Claude-Session: https://claude.ai/code/session_011ZyGVR3TdcTDLhuzyEbkBN --- .../src/builtin/connector-nodes.test.ts | 51 ++++- .../src/builtin/connector-nodes.ts | 7 +- .../src/builtin/wait-node.test.ts | 14 +- .../src/builtin/wait-node.ts | 7 +- .../connector-action-config-required.test.ts | 15 +- .../flow-sibling-block-loose-keys.ts | 21 +- .../spec/src/conversions/conversions.test.ts | 212 +++++++++++++++--- ...sibling-block-loose-config-keys-refused.ts | 43 ++++ .../retired-key-removal-version.pin.test.ts | 34 +++ 9 files changed, 354 insertions(+), 50 deletions(-) create mode 100644 packages/spec/src/migrations/entries/semantic/18.flow-sibling-block-loose-config-keys-refused.ts diff --git a/packages/services/service-automation/src/builtin/connector-nodes.test.ts b/packages/services/service-automation/src/builtin/connector-nodes.test.ts index 7a6e6f4f9cf..862b68aba66 100644 --- a/packages/services/service-automation/src/builtin/connector-nodes.test.ts +++ b/packages/services/service-automation/src/builtin/connector-nodes.test.ts @@ -125,10 +125,12 @@ describe('connector_action (baseline node)', () => { // #4045 — this proves the CONVERSION, not executor tolerance. The executor // reads only the declared `connectorConfig` block; the config-authored trio - // reaches it because `registerFlow` applies - // `flow-node-connector-config-lift`, which lifts it. This is the exact - // shape the descriptor's former configSchema mis-taught the schema-driven - // Studio form to write. + // reaches it because `registerFlow` — the rehydration seam a stored flow + // meets — replays `flow-node-connector-config-lift`, which lifts it. That + // entry left the authoring load path at 18 (#23543 F3): `defineStack` / + // `os validate` refuse this shape now, and only a stored row is lifted. + // This is the exact shape the descriptor's former configSchema mis-taught + // the schema-driven Studio form to write. it('accepts the mis-taught config.{connectorId,actionId,input} shape via the lift', async () => { let received: Record | undefined; engine.registerConnector(fakeConnector(), { @@ -163,6 +165,47 @@ describe('connector_action (baseline node)', () => { expect(received).toEqual({ message: 'hi' }); }); + // #23543 F3 — a stored row whose complete block shadows a loose key: the + // flow parse refuses every loose key under `config` since 18, so the lift + // deletes the shadowed one at this seam, and the row registers and runs + // with the block's input alone — never the stale loose one. + it('registers a stored row whose block shadows a loose `config.input`, and dispatches the block\'s input', async () => { + let received: Record | undefined; + engine.registerConnector(fakeConnector(), { + async echo(input) { + received = input; + return { ok: true }; + }, + }); + + engine.registerFlow('shadowed_flow', { + name: 'shadowed_flow', + label: 'Shadowed Flow', + type: 'autolaunched', + nodes: [ + { id: 'start', type: 'start', label: 'Start' }, + { + id: 'call', + type: 'connector_action', + label: 'Shadowed', + connectorConfig: { connectorId: 'fake', actionId: 'echo', input: { message: 'block' } }, + config: { input: { message: 'stale' } }, + }, + { id: 'end', type: 'end', label: 'End' }, + ], + edges: [ + { id: 'e1', source: 'start', target: 'call' }, + { id: 'e2', source: 'call', target: 'end' }, + ], + }); + + const node = (await engine.getFlow('shadowed_flow'))!.nodes.find((n) => n.id === 'call')!; + expect(node.config).toEqual({}); + const result = await engine.execute('shadowed_flow'); + expect(result.success).toBe(true); + expect(received).toEqual({ message: 'block' }); + }); + // #4045 — `input` is optional in the spec block because the executor // dispatches with `input ?? {}` and the designer's keyValue editor omits an // empty map entirely. Under the old required `input`, this exact designer diff --git a/packages/services/service-automation/src/builtin/connector-nodes.ts b/packages/services/service-automation/src/builtin/connector-nodes.ts index 817b75c77f1..7d548ff6324 100644 --- a/packages/services/service-automation/src/builtin/connector-nodes.ts +++ b/packages/services/service-automation/src/builtin/connector-nodes.ts @@ -46,9 +46,10 @@ export function registerConnectorNodes(engine: AutomationEngine, ctx: PluginCont // `config.` and REPLACING the client's hand-written // connectorConfig form (with its connector/action pickers). So the // schema steered online authoring to keys the executor never - // reads, and the node refused to dispatch. Stored flows that - // carry the mis-taught shape are healed at load by the ADR-0087 D2 - // conversion 'flow-node-connector-config-lift'. + // reads, and the node refused to dispatch. Since 18 the flow parse + // refuses those loose keys at authoring, and stored flows that + // carry the mis-taught shape are healed at rehydration by the + // retired ADR-0087 D2 conversion 'flow-node-connector-config-lift'. }), async execute(node, variables, context) { const cfg = node.connectorConfig; diff --git a/packages/services/service-automation/src/builtin/wait-node.test.ts b/packages/services/service-automation/src/builtin/wait-node.test.ts index 23aed47c228..ac4c65eb0b8 100644 --- a/packages/services/service-automation/src/builtin/wait-node.test.ts +++ b/packages/services/service-automation/src/builtin/wait-node.test.ts @@ -840,7 +840,9 @@ describe('wait timer one-shot vs. a shot that never consumed the pause (#5529)', * * These go through `registerFlow`, which is the seam that applies the conversion * — so they prove the graduation end-to-end on a legacy source, not merely that - * the executor stopped looking. Both spellings under test (`duration`, `signal`) + * the executor stopped looking. Since 18 (#23543 F3) that seam is the STORED + * flow's: the conversion left the authoring load path, and an authored loose key + * is refused by the flow parse, so these read as "a stored row still runs". Both spellings under test (`duration`, `signal`) * are ones the spec never declared; they existed only as the tail of the `??` * chains this change deleted. */ @@ -896,10 +898,10 @@ describe('wait config graduation — legacy loose `config` still works via the c expect(ran).toEqual(['after']); }); - // Unlike the two above, this one passes with the conversion unregistered too — - // it pins the EXECUTOR's side of the precedence (a declared value is what gets - // read), not the conversion's. Kept because that is the half a future - // "simplification" of the lift could silently invert. + // It pins the EXECUTOR's side of the precedence (a declared value is what gets + // read) — the half a future "simplification" of the lift could silently + // invert — and, since #23543 F3, the lift's: the shadowed loose key is + // deleted at this seam, or the stored row would not parse at all. it('reads the declared value, not its loose counterpart, when both are present', async () => { const { ctx, scheduled } = fakeJobCtx(); registerWaitNode(engine, ctx); @@ -919,6 +921,8 @@ describe('wait config graduation — legacy loose `config` still works via the c ], }); + const pause = (await engine.getFlow('wait_flow'))!.nodes.find((n) => n.id === 'pause')!; + expect(pause.config).toEqual({}); const before = Date.now(); await engine.execute('wait_flow'); // 1h (declared) — not 9h (loose). Same precedence the deleted `??` had. diff --git a/packages/services/service-automation/src/builtin/wait-node.ts b/packages/services/service-automation/src/builtin/wait-node.ts index 15bc7408059..aaba8ecada5 100644 --- a/packages/services/service-automation/src/builtin/wait-node.ts +++ b/packages/services/service-automation/src/builtin/wait-node.ts @@ -227,9 +227,10 @@ export function registerWaitNode(engine: AutomationEngine, ctx: PluginContext): // The loose `config.*` back door this used to also read — six keys, two of // them (`duration`, `signal`) spellings the spec never declared — graduated // into the ADR-0087 D2 conversion layer as - // `flow-node-wait-event-config-lift` (#4045). It is rewritten at load, - // including the `registerFlow` rehydration seam, so there is nothing left - // to fall back to here (PD #12: no consumer-side fallbacks). + // `flow-node-wait-event-config-lift` (#4045). Since 18 the flow parse + // refuses those keys at authoring, and a stored flow's are rewritten at + // the `registerFlow` rehydration seam, so there is nothing left to fall + // back to here (PD #12: no consumer-side fallbacks). // // ── REVERSAL (#17928) ──────────────────────────────────────────────── // Until this change the two lines below read `(node.waitEventConfig ?? {})` diff --git a/packages/spec/src/automation/connector-action-config-required.test.ts b/packages/spec/src/automation/connector-action-config-required.test.ts index 2196c444169..a42cdf7289d 100644 --- a/packages/spec/src/automation/connector-action-config-required.test.ts +++ b/packages/spec/src/automation/connector-action-config-required.test.ts @@ -58,13 +58,18 @@ describe('FlowSchema refuses a connector_action node with no connectorConfig blo ]); }); - it('the keys written under `config` instead: refused at the block, and told to move them', () => { - // A direct parse meets the pre-conversion spelling, like every other - // tombstone; `registerFlow` and `objectstack validate` convert a complete - // pair into the block first (the `flow-node-connector-config-lift` D2 entry). + it('the keys written under `config` instead: refused at the block and at each loose key, and told to move them', () => { + // Every authoring door meets this spelling as written: the + // `flow-node-connector-config-lift` D2 entry left the load path at 18 + // (#23543 F3), and only a stored row's rehydration seam still lifts it. const issues = issuesOf(flowWith(connector({ config: { connectorId: 'slack', actionId: 'chat.postMessage' } }))); - expect(issues.map((i) => [i.code, i.path])).toEqual([['custom', ['nodes', 1, 'connectorConfig']]]); + expect(issues.map((i) => [i.code, i.path])).toEqual([ + ['custom', ['nodes', 1, 'connectorConfig']], + ['custom', ['nodes', 1, 'config', 'connectorId']], + ['custom', ['nodes', 1, 'config', 'actionId']], + ]); expect(issues[0].message).toContain('from `config` into the block'); + expect(issues[1].message).toContain('Move it to `connectorConfig.connectorId`'); }); }); diff --git a/packages/spec/src/automation/flow-sibling-block-loose-keys.ts b/packages/spec/src/automation/flow-sibling-block-loose-keys.ts index 8bc10e040ce..b358a8aa7cd 100644 --- a/packages/spec/src/automation/flow-sibling-block-loose-keys.ts +++ b/packages/spec/src/automation/flow-sibling-block-loose-keys.ts @@ -64,6 +64,23 @@ function looseKeyTargets(type: unknown): ReadonlyMap | undefined return undefined; } +/** + * Where a loose key goes. `timeoutMs` is the one key whose block spelling is + * itself retired (`waitEventConfig.timeoutMs`, removed at 17 because it was only + * ever read as the timer duration), so its move is the one the chain makes for + * it: onto `timerDuration`, as a string. + */ +function looseKeyMove(block: string, key: string, target: string): string { + if (block === 'waitEventConfig' && key === 'timeoutMs') { + return 'It was only ever read as the timer duration in milliseconds: move it to `waitEventConfig.timerDuration` ' + + "as a string (`timeoutMs: 60000` → `timerDuration: '60000'`, or `'PT1M'`). If `waitEventConfig.timerDuration` " + + 'is already set, it is the one the node runs with — delete `config.timeoutMs`.'; + } + const rename = target === key ? '' : ` (the key is \`${target}\` there; \`${key}\` was only ever an executor fallback)`; + return `Move it to \`${block}.${target}\`${rename}; the value is unchanged. If \`${block}.${target}\` is already set, ` + + `it is the one the node runs with — delete \`config.${key}\`.`; +} + /** * Every named loose key under a `wait` / `connector_action` node's `config`, * each refused at `config.` with the prescription naming the sibling block @@ -83,14 +100,12 @@ export function siblingBlockLooseConfigRefusals(node: unknown): Array<{ path: Pr for (const key of Object.keys(node.config)) { const target = targets.get(key); if (target === undefined || node.config[key] === undefined) continue; - const rename = target === key ? '' : ` (the key is \`${target}\` there; \`${key}\` was only ever an executor fallback)`; out.push({ path: ['config', key], message: `A \`${String(node.type)}\` node's \`config.${key}\` was removed in @objectstack/spec 18 (ADR-0087 D2) — this ` + `node type has no \`config\` contract: its executor reads only the \`${block}\` block, and the loader lifted ` - + `this key into it until this release. Move it to \`${block}.${target}\`${rename}; the value is unchanged. If ` - + `\`${block}.${target}\` is already set, it is the one the node runs with — delete \`config.${key}\`. ` + + `this key into it until this release. ${looseKeyMove(block, key, target)} ` + MIGRATE_META_SENTENCE, }); } diff --git a/packages/spec/src/conversions/conversions.test.ts b/packages/spec/src/conversions/conversions.test.ts index 646185057c9..b600d8b724b 100644 --- a/packages/spec/src/conversions/conversions.test.ts +++ b/packages/spec/src/conversions/conversions.test.ts @@ -16,7 +16,7 @@ import { ElementButtonPropsSchema, PageHeaderProps, PageTabsProps } from '../ui/ import { PageSchema } from '../ui/page.zod.js'; import { defineStack } from '../stack.zod.js'; import { applyMetaMigrations } from '../migrations/chain.js'; -import { applyConversions, collectConversionNotices } from './apply.js'; +import { applyConversions, applyConversionsToFlow, collectConversionNotices } from './apply.js'; import { ALL_CONVERSIONS, CONVERSIONS_BY_MAJOR } from './registry.js'; import { applyConversionsToStoredItem } from './stored.js'; import { renameConfigKey, renameKey } from './walk.js'; @@ -400,9 +400,7 @@ describe('conversion layer (ADR-0087 D2)', () => { it('every entry whose window ran out before 18 is retired or kept by name', () => { const kept = ALL_CONVERSIONS.filter((c) => c.toMajor < 18 && !c.retiredFromLoadPath).map((c) => c.id).sort(); expect(kept).toEqual([ - 'flow-node-connector-config-lift', 'flow-node-http-callout-rename', - 'flow-node-wait-event-config-lift', 'page-component-visibility-to-visibleWhen', 'retry-policy-converged', 'view-visibleOn-to-visibleWhen', @@ -419,6 +417,10 @@ describe('conversion layer (ADR-0087 D2)', () => { 'datasource-driver-mongo-to-mongodb', // #23543 F5-jsx: retired once `jsx` itself was retired as a page kind. 'page-kind-jsx-to-html', + // #23543 F3: retired once the flow parse refused the loose `config` + // keys and each lift deleted the keys its block shadows. + 'flow-node-wait-event-config-lift', + 'flow-node-connector-config-lift', ]) { expect(ALL_CONVERSIONS.find((c) => c.id === id), id).toMatchObject({ retiredFromLoadPath: true, retiredAfter: '17.7.0' }); } @@ -851,6 +853,10 @@ describe('conversion layer (ADR-0087 D2)', () => { }); }); + // #23543 F3 retired both sibling-block lifts at 18. Every lift case below + // replays the chain the way a stored flow meets it (`includeRetired`, the + // automation engine's rehydration seam); the last describe pins the three + // claims the retirement makes together. describe('flow-node-wait-event-config-lift (PD #12 retirement)', () => { /** * One `wait` node in a flow that `FlowSchema` can actually parse — `label` is @@ -872,45 +878,55 @@ describe('conversion layer (ADR-0087 D2)', () => { ], }); const waitNodeOf = (stack: Record) => (stack.flows as any[])[0].nodes[1]; + const stored = (stack: Record) => collectConversionNotices(stack, { includeRetired: true }); it('lifts the undeclared `duration` / `signal` spellings onto the declared block', () => { - const { stack, notices } = collectConversionNotices(waitFlow({ config: { duration: 'PT1M' } })); + const { stack, notices } = stored(waitFlow({ config: { duration: 'PT1M' } })); expect(waitNodeOf(stack).waitEventConfig).toEqual({ timerDuration: 'PT1M', eventType: 'timer' }); expect(waitNodeOf(stack).config).toEqual({}); expect(notices).toHaveLength(1); - const sig = collectConversionNotices(waitFlow({ config: { eventType: 'signal', signal: 'order_paid' } })); + const sig = stored(waitFlow({ config: { eventType: 'signal', signal: 'order_paid' } })); expect(waitNodeOf(sig.stack).waitEventConfig).toEqual({ eventType: 'signal', signalName: 'order_paid' }); expect(sig.notices).toHaveLength(2); }); - it('lets a declared value win and leaves its loose counterpart shadowed in place', () => { - const { stack, notices } = collectConversionNotices( + it('lets a declared value win and deletes its shadowed loose counterpart', () => { + const { stack, notices } = stored( waitFlow({ waitEventConfig: { eventType: 'timer', timerDuration: 'PT5M' }, - config: { duration: 'PT9M', timeoutMs: 60_000 }, + // (Not `timeoutMs` as the second key: with the full chain replayed, + // protocol 17's `flow-node-wait-timeout-keys-removed` deletes it + // from the block again.) + config: { duration: 'PT9M', signalName: 'order_paid' }, }), ); expect(waitNodeOf(stack).waitEventConfig).toEqual({ eventType: 'timer', timerDuration: 'PT5M', - timeoutMs: 60_000, + signalName: 'order_paid', }); - // The loose counterpart is not deleted. NOTE this is the wait-node LIFT - // (a loose key into a declared block), not a `renameKey` alias pair, so - // #4923's by-value split does not reach it — see the comment on - // WAIT_EVENT_CONFIG_LIFTS. - expect(waitNodeOf(stack).config).toEqual({ duration: 'PT9M' }); - expect(notices).toHaveLength(1); + // #23543 F3: the shadowed loose key is deleted, not left in place — the + // flow parse refuses every named loose key since 18, so a kept one would + // stop the stored flow loading. One notice per key: the lift and the drop. + expect(waitNodeOf(stack).config).toEqual({}); + expect(notices.map((n) => n.to)).toEqual([ + '(removed — `waitEventConfig.timerDuration` already set, so it was never read)', + 'waitEventConfig.signalName', + ]); }); - it('prefers the declared spelling over the undeclared one among loose keys', () => { - const { stack } = collectConversionNotices( - waitFlow({ config: { timerDuration: 'PT1H', duration: 'PT2H' } }), - ); + it('prefers the declared spelling over the undeclared one among loose keys, and drops the loser', () => { + const { stack } = stored(waitFlow({ config: { timerDuration: 'PT1H', duration: 'PT2H' } })); expect(waitNodeOf(stack).waitEventConfig.timerDuration).toBe('PT1H'); - // `duration` was never reachable past the `??` chain either — left as-is. - expect(waitNodeOf(stack).config).toEqual({ duration: 'PT2H' }); + // `duration` was never reachable past the `??` chain either — deleted. + expect(waitNodeOf(stack).config).toEqual({}); + }); + + it('a loose `null` is skipped by the precedence and deleted, as the `??` chain skipped it', () => { + const { stack } = stored(waitFlow({ config: { timerDuration: null, duration: 'PT2H', note: 'keep me' } })); + expect(waitNodeOf(stack).waitEventConfig).toEqual({ timerDuration: 'PT2H', eventType: 'timer' }); + expect(waitNodeOf(stack).config).toEqual({ note: 'keep me' }); }); /** @@ -919,9 +935,7 @@ describe('conversion layer (ADR-0087 D2)', () => { * the combination the ledger's candidate order has to get right. */ it('lifts the showcase shape: declared key names in a loose `config`', () => { - const { stack, notices } = collectConversionNotices( - waitFlow({ config: { eventType: 'signal', signalName: 'budget_revision' } }), - ); + const { stack, notices } = stored(waitFlow({ config: { eventType: 'signal', signalName: 'budget_revision' } })); expect(waitNodeOf(stack).waitEventConfig).toEqual({ eventType: 'signal', signalName: 'budget_revision', @@ -933,13 +947,13 @@ describe('conversion layer (ADR-0087 D2)', () => { it('leaves a wait node with nothing to lift completely untouched', () => { const before = waitFlow({ waitEventConfig: { eventType: 'manual' }, config: { note: 'keep me' } }); - const { stack, notices } = collectConversionNotices(structuredClone(before)); + const { stack, notices } = stored(structuredClone(before)); expect(waitNodeOf(stack)).toEqual(waitNodeOf(before as any)); expect(notices).toHaveLength(0); }); it('only touches `wait` — the same keys on another node type are not its surface', () => { - const { stack, notices } = collectConversionNotices({ + const { stack, notices } = stored({ flows: [{ name: 'f', nodes: [{ id: 'a', type: 'custom', config: { duration: 'PT1M' } }] }], }); expect((stack.flows as any[])[0].nodes[0].config).toEqual({ duration: 'PT1M' }); @@ -954,7 +968,7 @@ describe('conversion layer (ADR-0087 D2)', () => { * default does not. */ it('produces a flow the loader can still parse (negative control: no eventType would not)', () => { - const { stack } = collectConversionNotices(waitFlow({ config: { duration: 'PT1M' } })); + const { stack } = stored(waitFlow({ config: { duration: 'PT1M' } })); const converted = (stack.flows as any[])[0]; expect(() => FlowSchema.parse(converted)).not.toThrow(); @@ -964,6 +978,150 @@ describe('conversion layer (ADR-0087 D2)', () => { }); }); + describe('flow-node-connector-config-lift (stored rows)', () => { + const connectorFlow = (node: Record) => ({ + flows: [ + { + name: 'notify_slack', + label: 'Notify Slack', + type: 'autolaunched', + edges: [], + nodes: [ + { id: 'n1', type: 'start', label: 'Start' }, + { id: 'c', type: 'connector_action', label: 'Call', ...node }, + ], + }, + ], + }); + const nodeOf = (stack: Record) => (stack.flows as any[])[0].nodes[1]; + const stored = (stack: Record) => collectConversionNotices(stack, { includeRetired: true }); + + it('lifts the mis-taught trio into the block', () => { + const { stack, notices } = stored( + connectorFlow({ config: { connectorId: 'slack', actionId: 'chat.postMessage', input: { text: 'Hi' } } }), + ); + expect(nodeOf(stack).connectorConfig).toEqual({ connectorId: 'slack', actionId: 'chat.postMessage', input: { text: 'Hi' } }); + expect(nodeOf(stack).config).toEqual({}); + expect(notices).toHaveLength(3); + expect(FlowSchema.safeParse((stack.flows as any[])[0]).success).toBe(true); + }); + + it('deletes a loose key the complete block shadows, and lifts the one it does not', () => { + const { stack, notices } = stored( + connectorFlow({ + connectorConfig: { connectorId: 'rest', actionId: 'get' }, + config: { connectorId: 'ignored', input: { path: '/health' }, note: 'keep me' }, + }), + ); + expect(nodeOf(stack).connectorConfig).toEqual({ connectorId: 'rest', actionId: 'get', input: { path: '/health' } }); + expect(nodeOf(stack).config).toEqual({ note: 'keep me' }); + expect(notices.map((n) => n.from).sort()).toEqual(['config.connectorId', 'config.input']); + }); + + it('the completeness guard leaves a node it cannot complete untouched — it fails to load either way', () => { + const before = connectorFlow({ config: { connectorId: 'slack' } }); + const { stack, notices } = stored(structuredClone(before)); + expect(nodeOf(stack)).toEqual(nodeOf(before)); + expect(notices).toHaveLength(0); + const parsed = FlowSchema.safeParse((stack.flows as any[])[0]); + expect(parsed.success).toBe(false); + expect(parsed.error!.issues.map((i) => i.path.join('.'))).toEqual(['nodes.1.connectorConfig', 'nodes.1.config.connectorId']); + }); + }); + + // #23543 F3 — the retirement's three claims, for both node types: a STORED + // flow (the `includeRetired` seams) still loads, shadowed loose key and all; + // the authoring funnel no longer rewrites an authored loose key; and an + // authored loose key meets the prescription naming its block key instead. + describe('the sibling-block lifts are retired (#23543 F3)', () => { + const flowOf = (node: Record) => ({ + name: 'settle', + label: 'Settle', + type: 'autolaunched', + edges: [], + nodes: [ + { id: 'n1', type: 'start', label: 'Start' }, + { label: 'Step', ...node }, + ], + }); + const LOOSE_WAIT = { id: 'w', type: 'wait', config: { eventType: 'signal', signal: 'order_paid' } }; + const SHADOWED_WAIT = { + id: 'w', + type: 'wait', + waitEventConfig: { eventType: 'timer', timerDuration: 'PT5M' }, + config: { duration: 'PT9M' }, + }; + const LOOSE_CONNECTOR = { id: 'c', type: 'connector_action', config: { connectorId: 'slack', actionId: 'chat.postMessage' } }; + const SHADOWED_CONNECTOR = { + id: 'c', + type: 'connector_action', + connectorConfig: { connectorId: 'slack', actionId: 'chat.postMessage', input: { text: 'Hi' } }, + config: { input: { text: 'stale' } }, + }; + + it.each([ + ['a loose-only wait row', LOOSE_WAIT, { waitEventConfig: { eventType: 'signal', signalName: 'order_paid' }, config: {} }], + ['a wait row whose block shadows a loose key', SHADOWED_WAIT, { waitEventConfig: SHADOWED_WAIT.waitEventConfig, config: {} }], + ['a loose-only connector row', LOOSE_CONNECTOR, { connectorConfig: LOOSE_CONNECTOR.config, config: {} }], + ['a connector row whose block shadows a loose key', SHADOWED_CONNECTOR, { connectorConfig: SHADOWED_CONNECTOR.connectorConfig, config: {} }], + ])('%s still loads at the rehydration seam, with the block alone', (_name, node, expected) => { + const raw = flowOf(node); + // The stored row as written is what the parse now refuses … + expect(FlowSchema.safeParse(raw).success).toBe(false); + // … and the seam's replay (the automation engine's call) makes it load. + const converted = applyConversionsToFlow(structuredClone(raw), { includeRetired: true }) as ReturnType; + expect(converted.nodes[1]).toMatchObject(expected); + expect(FlowSchema.safeParse(converted).success).toBe(true); + }); + + it('is retired from the load path — the authoring funnel no longer lifts an authored loose key', () => { + for (const node of [LOOSE_WAIT, SHADOWED_WAIT, LOOSE_CONNECTOR, SHADOWED_CONNECTOR]) { + const before = { flows: [flowOf(node)] }; + const { stack, notices } = collectConversionNotices(structuredClone(before)); + expect(stack, node.type).toEqual(before); + expect(notices, node.type).toHaveLength(0); + } + }); + + it.each([ + ['wait', SHADOWED_WAIT, 'nodes.1.config.duration', "A `wait` node's `config.duration` was removed in @objectstack/spec 18", '`waitEventConfig.timerDuration`'], + ['connector_action', SHADOWED_CONNECTOR, 'nodes.1.config.input', "A `connector_action` node's `config.input` was removed in @objectstack/spec 18", '`connectorConfig.input`'], + ])('an authored loose key on a %s node reaches defineStack as written and is refused naming its block key', (_type, node, path, first, target) => { + let refusal: unknown; + try { + defineStack({ flows: [flowOf(node)] } as never); + } catch (error) { + refusal = error; + } + expect(refusal).toMatchObject({ code: 'STACK_SCHEMA_INVALID', status: 422 }); + const issues = (refusal as { issues: { path: PropertyKey[]; message: string }[] }).issues; + // The loose key is the ONE issue: the block is complete, so the refusal + // is about the location alone. + expect(issues.map((i) => i.path.join('.'))).toEqual([`flows.0.${path}`]); + expect(issues[0]!.message.startsWith(first)).toBe(true); + expect(issues[0]!.message).toContain(`Move it to ${target}`); + }); + + it('refuses every named loose key and no other — the table is the lifts\' own', () => { + const parsed = FlowSchema.safeParse(flowOf({ + id: 'w', + type: 'wait', + waitEventConfig: { eventType: 'timer', timerDuration: 'PT5M' }, + config: { eventType: 'timer', timerDuration: 'PT1M', duration: 'PT1M', signalName: 's', signal: 's', timeoutMs: 1, note: 'not a lifted key' }, + })); + expect(parsed.success).toBe(false); + expect(parsed.error!.issues.map((i) => i.path.slice(3).join('.')).sort()).toEqual( + ['duration', 'eventType', 'signal', 'signalName', 'timeoutMs', 'timerDuration'], + ); + // CONTROL — the declared blocks alone parse, and the same keys under + // another node type's `config` are not this refusal's. + expect(FlowSchema.safeParse(flowOf({ id: 'w', type: 'wait', waitEventConfig: { eventType: 'timer', timerDuration: 'PT5M' } })).success).toBe(true); + expect(FlowSchema.safeParse(flowOf({ id: 'c', type: 'connector_action', connectorConfig: SHADOWED_CONNECTOR.connectorConfig })).success).toBe(true); + const other = FlowSchema.safeParse(flowOf({ id: 'a', type: 'assignment', config: { input: 1, duration: 'PT1M' } })); + expect(other.success ? [] : other.error.issues.map((i) => i.message)).not.toContainEqual(expect.stringContaining('was removed in @objectstack/spec 18')); + }); + }); + // Commit e2798fab7 renamed the canonical id `mongo` → `mongodb`; #23543 F1 // retired `mongo` as a value at 18. Three claims hold together: the stored // value still CONVERGES (the data-at-rest seams replay the retired entry), an diff --git a/packages/spec/src/migrations/entries/semantic/18.flow-sibling-block-loose-config-keys-refused.ts b/packages/spec/src/migrations/entries/semantic/18.flow-sibling-block-loose-config-keys-refused.ts new file mode 100644 index 00000000000..6520e360cf4 --- /dev/null +++ b/packages/spec/src/migrations/entries/semantic/18.flow-sibling-block-loose-config-keys-refused.ts @@ -0,0 +1,43 @@ +// Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license. + +import type { SemanticMigration } from '../../types.js'; + +// #23543 F3 — the two lifts' one D3 entry. The D2 halves are +// `flow-node-wait-event-config-lift` and `flow-node-connector-config-lift` +// (both step-17 conversions), which move every named loose key the chain can +// reach into its block and are now retired from the load path; this entry +// states the refusal an authored source meets at 18, and the one shape no +// conversion repairs — a connector node whose pair the lift cannot complete. +export const entry: SemanticMigration = { + id: 'flow-sibling-block-loose-config-keys-refused', + // No backticks and no pipes in `surface` — build-upgrade-guide.ts renders it + // inside a code span AND a table cell. + surface: + 'wait and connector_action flow-node settings written under config (eventType, timerDuration, duration, ' + + 'signalName, signal, timeoutMs on wait; connectorId, actionId, input on connector_action)', + replacement: + 'the sibling block each executor reads — `waitEventConfig` (`duration` → `timerDuration`, `signal` → ' + + '`signalName`, `timeoutMs` → `timerDuration` as a string, the other keys by the same name) and ' + + '`connectorConfig` (each key by the same name)', + reason: + 'A maintainer ruling retired the two lifts at 18. Neither node type has a `config` contract: the `wait` ' + + 'executor reads only `waitEventConfig` and the `connector_action` executor only `connectorConfig`. ' + + 'Through 17 the loader lifted these keys from `config` into the block with a notice. At 18 the flow ' + + 'parse refuses each one where it is written, naming the block key it belongs on, at `objectstack ' + + 'validate`, `objectstack build` and `defineStack`, so under `objectstack lint` too when the config ' + + 'calls it — because with the lift gone a loose key would be read by nobody, and a loose `input` ' + + 'beside a complete `connectorConfig` would dispatch the action with no input. The D2 conversions ' + + 'make the move wherever the chain reaches: `objectstack migrate meta` lists every such key in your ' + + 'metadata sources. Where the block already sets the same key, the block is the one the node always ' + + 'ran with, and the loose key is deleted. Stored flows keep loading: every rehydration seam replays ' + + 'both conversions, and each now deletes the loose keys its block shadows instead of leaving them ' + + 'for the parse to refuse; the metadata save door judges a flow after the same replay, so a script ' + + 'that still sends the loose spelling keeps saving. What no conversion repairs is a `connector_action` ' + + 'node whose `connectorId` / `actionId` pair is complete in neither place: that node never loaded, ' + + 'and it is still refused until its block names both.', + acceptanceCriteria: + '`objectstack validate` reports no refusal at a `config` path of a `wait` or `connector_action` node, ' + + 'every such node declares its settings in `waitEventConfig` / `connectorConfig` only, and each wait ' + + 'resumes and each connector action dispatches with the same input as before.', + conversionIds: ['flow-node-wait-event-config-lift', 'flow-node-connector-config-lift'], +}; diff --git a/packages/spec/src/shared/retired-key-removal-version.pin.test.ts b/packages/spec/src/shared/retired-key-removal-version.pin.test.ts index fa3fea1da37..e09587dab8f 100644 --- a/packages/spec/src/shared/retired-key-removal-version.pin.test.ts +++ b/packages/spec/src/shared/retired-key-removal-version.pin.test.ts @@ -90,6 +90,7 @@ import { AdminScopeSchema } from '../security/permission.zod'; import { SharingLevel } from '../security/sharing.zod'; import { DatasourceSchema } from '../data/datasource.zod'; import { PageSchema } from '../ui/page.zod'; +import { FlowSchema } from '../automation/flow.zod'; import { PROTOCOL_MAJOR } from '../kernel/protocol-version'; import { EmailTemplateDefinitionSchema } from '../system/email-template.zod'; import { @@ -217,6 +218,15 @@ const ELEMENTS: readonly (readonly [unknown, string, readonly string[]])[] = [ [ElementRepeaterPropsSchema, 'element:repeater', ['object', 'filter', 'sort', 'limit']], ]; +/** A parseable flow holding one `node` after its start node. */ +const flowWithNode = (node: Record) => ({ + name: 'settle', + label: 'Settle', + type: 'autolaunched', + edges: [], + nodes: [{ id: 'n1', type: 'start', label: 'Start' }, { id: 'n2', label: 'Step', ...node }], +}); + /** A notify config that is valid on its own, so a retired key is its only issue. */ const NOTIFY_BASE = { recipients: ['u1'], title: 'Hi' }; @@ -320,6 +330,30 @@ const JOIN: Readonly> = { 'kind', ), ], + // #23543 F3: a loose `config` key under a sibling-block node type, refused by + // the flow parse; the node's block is complete, so the key is the only issue. + 'flow-node-wait-event-config-lift': [ + refusal( + "a wait node's loose config.duration", + () => FlowSchema.safeParse(flowWithNode({ + type: 'wait', + waitEventConfig: { eventType: 'timer', timerDuration: 'PT5M' }, + config: { duration: 'PT9M' }, + })), + 'nodes', + ), + ], + 'flow-node-connector-config-lift': [ + refusal( + "a connector_action node's loose config.input", + () => FlowSchema.safeParse(flowWithNode({ + type: 'connector_action', + connectorConfig: { connectorId: 'slack', actionId: 'chat.postMessage' }, + config: { input: { text: 'Hi' } }, + })), + 'nodes', + ), + ], // Shipped by a 17.x minor; `toMajor: 18` like the rows above. 'page-header-breadcrumb-removed': [tombstone(PageHeaderProps, 'page:header', 'breadcrumb')], 'object-grid-default-sort-removed': [tombstone(ObjectGridPropsSchema, 'object-grid', 'defaultSort')], From b8622ad1e4b7cb5e9f41af0df4e154a212e57220 Mon Sep 17 00:00:00 2001 From: Claude Date: Thu, 15 Oct 2026 22:28:01 +0000 Subject: [PATCH 3/4] chore(changeset): F3 BREAKING changeset Co-authored-by: Claude Claude-Session: https://claude.ai/code/session_011ZyGVR3TdcTDLhuzyEbkBN --- ...43-f3-retire-wait-connector-config-lift.md | 26 +++++++++++++++++++ 1 file changed, 26 insertions(+) create mode 100644 .changeset/23543-f3-retire-wait-connector-config-lift.md diff --git a/.changeset/23543-f3-retire-wait-connector-config-lift.md b/.changeset/23543-f3-retire-wait-connector-config-lift.md new file mode 100644 index 00000000000..67b1c7412b0 --- /dev/null +++ b/.changeset/23543-f3-retire-wait-connector-config-lift.md @@ -0,0 +1,26 @@ +--- +'@objectstack/spec': minor +'@objectstack/service-automation': patch +--- + +feat(spec)!: the loose `config` keys of a `wait` or `connector_action` flow node are refused, naming the `waitEventConfig` / `connectorConfig` key they belong on, and `flow-node-wait-event-config-lift` and `flow-node-connector-config-lift` leave the load path + +Clause-②: yes (narrowing) + +**BREAKING (authoring):** neither node type has a `config` contract: the `wait` executor reads only `waitEventConfig` and the `connector_action` executor only `connectorConfig`. Through 17 the loader lifted settings written under the node's `config` into that block with a notice. At 18 both ADR-0087 D2 conversions are retired from the load path (`retiredAfter: '17.7.0'`), and the flow parse refuses each such key where it is written, at every door that judges a flow (`defineStack`, `os validate`, `os build`, and `os lint` when the config calls it), a node inside a loop / parallel / try-catch region body included: "A `wait` node's `config.duration` was removed in @objectstack/spec 18 (ADR-0087 D2) … Move it to `waitEventConfig.timerDuration` …". The refused keys are exactly the ones the lifts moved: `eventType`, `timerDuration`, `duration`, `signalName`, `signal` and `timeoutMs` on `wait`; `connectorId`, `actionId` and `input` on `connector_action`. Any other `config` key on these node types is judged as before. + +Migration: + +- `wait` node `config.eventType` / `config.timerDuration` / `config.signalName` → the same key under `waitEventConfig`. +- `wait` node `config.duration` → `waitEventConfig.timerDuration`, and `config.signal` → `waitEventConfig.signalName`. +- `wait` node `config.timeoutMs` → `waitEventConfig.timerDuration`, as a string (`60000` → `'60000'`, or `'PT1M'`): it was only ever read as the timer duration. +- `connector_action` node `config.connectorId` / `config.actionId` / `config.input` → the same key under `connectorConfig`. +- If the block already sets the same key, the block's value is the one the node always ran with: delete the loose key. + +Run `os migrate meta --from 16` to list the mechanical edits for existing sources; `--write` applies the ones it can prove, and you apply the rest by hand. The D3 entry `flow-sibling-block-loose-config-keys-refused` states the edit for upgraders. + +Stored metadata needs no edit. The automation engine's flow rehydration (`registerFlow`), stored-row rehydration and the metadata save door's flow verdict still replay both conversions, and each now DELETES a loose key its block already shadows (and a loose key holding `null`) instead of leaving it in place, so a `sys_metadata` flow carrying either shape keeps loading with the block alone. A `connector_action` node whose `connectorId` / `actionId` pair is complete in neither place is refused as before: no conversion can complete it. + +`@objectstack/service-automation`: comment-only changes in the `wait` and `connector_action` executors, which already read only their declared blocks. + + From cb5a4f9f58296e125b6622b808ff0cc3421d3d6b Mon Sep 17 00:00:00 2001 From: Claude Date: Thu, 15 Oct 2026 22:35:36 +0000 Subject: [PATCH 4/4] test(lint,service-automation): re-spell fixtures that used the retired loose config location; docs: name the refusal Co-authored-by: Claude Claude-Session: https://claude.ai/code/session_011ZyGVR3TdcTDLhuzyEbkBN --- content/docs/automation/flows.mdx | 4 ++-- packages/lint/src/lint-flow-patterns.test.ts | 4 ++-- .../lint/src/validate-expressions.flow-cel-root.test.ts | 4 ++-- .../services/service-automation/src/run-summary.test.ts | 6 +++--- 4 files changed, 9 insertions(+), 9 deletions(-) diff --git a/content/docs/automation/flows.mdx b/content/docs/automation/flows.mdx index f71dd588b46..5e9042fcfc3 100644 --- a/content/docs/automation/flows.mdx +++ b/content/docs/automation/flows.mdx @@ -139,11 +139,11 @@ Each node performs a specific action in the flow. | `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 — its node type owns its shape. On every built-in node type with an executor contract, a key that contract does not declare is refused at the flow parse, at the key's path; 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 | +| `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. The same three keys written under the node's `config` are refused (since 18), each naming its `connectorConfig` key | | `position` | `{ x, y }` | optional | Visual position on canvas | | `timeoutMs` | `number` | optional | Per-node execution timeout | | `inputSchema` | `object` | optional | Declared input parameter types, for Studio form generation and runtime validation | -| `waitEventConfig` | `object` | ✅ on `wait` | `wait`-node event descriptor (`eventType`, `timerDuration`, `signalName`). `FlowSchema` refuses a `wait` node without it; `eventType` has no default, and `eventType: 'timer'` requires a non-blank `timerDuration`. `timeoutMs` / `onTimeout` were removed in 17 (#4158) — `wait` has no timeout; `timerDuration` is a string, and a quoted bare number (`'60000'`) is read as milliseconds | +| `waitEventConfig` | `object` | ✅ on `wait` | `wait`-node event descriptor (`eventType`, `timerDuration`, `signalName`). `FlowSchema` refuses a `wait` node without it; `eventType` has no default, and `eventType: 'timer'` requires a non-blank `timerDuration`. `timeoutMs` / `onTimeout` were removed in 17 (#4158) — `wait` has no timeout; `timerDuration` is a string, and a quoted bare number (`'60000'`) is read as milliseconds. Event keys written under the node's `config` instead (`eventType`, `timerDuration` or `duration`, `signalName` or `signal`, `timeoutMs`) are refused (since 18), each naming its `waitEventConfig` key | | `boundaryConfig` | `object` | ✅ on `boundary_event` | BPMN boundary-event descriptor (interop). `FlowSchema` refuses a `boundary_event` node without it | diff --git a/packages/lint/src/lint-flow-patterns.test.ts b/packages/lint/src/lint-flow-patterns.test.ts index 6924c22d0d2..7b5ee06076d 100644 --- a/packages/lint/src/lint-flow-patterns.test.ts +++ b/packages/lint/src/lint-flow-patterns.test.ts @@ -287,7 +287,7 @@ describe('lintFlowPatterns — approval revise loop (ADR-0044)', () => { ) => { const stack = approvalFlow(edges) as any; stack.flows[0].nodes = stack.flows[0].nodes.map((n: any) => - n.id === 'wait' ? { id: 'wait', type: 'wait', config: { eventType: 'signal' } } : n, + n.id === 'wait' ? { id: 'wait', type: 'wait', waitEventConfig: { eventType: 'signal' } } : n, ); return stack; }; @@ -2089,7 +2089,7 @@ describe('per-iteration containment (#13681 / #14394)', () => { { id: 'n4', type: 'delete_record', label: 'Delete', config: { objectName: 'task', filter: { case_id: CEL('currentCase.id') } } }, { id: 'n5', type: 'http', label: 'Call', config: { url: 'https://example.test' } }, { id: 'n6', type: 'notify', label: 'Notify', config: { title: 'x' } }, - { id: 'n7', type: 'connector_action', label: 'Connector', config: { connectorId: 'c', action: 'a' } }, + { id: 'n7', type: 'connector_action', label: 'Connector', connectorConfig: { connectorId: 'c', actionId: 'a' } }, { id: 'n8', type: 'script', label: 'Script', config: { function: 'f' } }, { id: 'n9', type: 'subflow', label: 'Subflow', config: { flowName: 'child' } }, { id: 'n10', type: 'map', label: 'Map', config: { collection: { dialect: 'cel', source: 'items' } } }, diff --git a/packages/lint/src/validate-expressions.flow-cel-root.test.ts b/packages/lint/src/validate-expressions.flow-cel-root.test.ts index a7355ad2416..c735855a631 100644 --- a/packages/lint/src/validate-expressions.flow-cel-root.test.ts +++ b/packages/lint/src/validate-expressions.flow-cel-root.test.ts @@ -209,8 +209,8 @@ describe('controls — one per binding kind the run has', () => { describe('the door stands down where the run can bind a name it cannot see', () => { it.each<[string, AnyRec]>([ ['a `script` (its function gets the live variable map)', { id: 'fn', type: 'script', config: { function: 'enrich' } }], - ['a `connector_action` (its handler gets the live variable map)', { id: 'push', type: 'connector_action', config: { connectorId: 'slack', actionId: 'post' } }], - ['a `wait` (a resume folds the caller\'s bag)', { id: 'hold', type: 'wait', config: { eventType: 'signal', signalName: 'go' } }], + ['a `connector_action` (its handler gets the live variable map)', { id: 'push', type: 'connector_action', connectorConfig: { connectorId: 'slack', actionId: 'post' } }], + ['a `wait` (a resume folds the caller\'s bag)', { id: 'hold', type: 'wait', waitEventConfig: { eventType: 'signal', signalName: 'go' } }], ['a `subflow` (a resume folds the caller\'s bag)', { id: 'child', type: 'subflow', config: { flowName: 'other' } }], ['a `map` (a resume folds the caller\'s bag)', { id: 'each', type: 'map', config: { collection: { dialect: 'cel', source: 'rows' }, flowName: 'other' } }], ['a plugin node type', { id: 'ai', type: 'ai_classify', config: {} }], diff --git a/packages/services/service-automation/src/run-summary.test.ts b/packages/services/service-automation/src/run-summary.test.ts index 8a43b1edfa3..168e5c49408 100644 --- a/packages/services/service-automation/src/run-summary.test.ts +++ b/packages/services/service-automation/src/run-summary.test.ts @@ -943,7 +943,7 @@ describe('uncountable effects (#4354 follow-up)', () => { name: 'f', label: 'f', type: 'autolaunched', nodes: [ { id: 'start', type: 'start', label: 'S' }, - { id: 'push', type: 'connector_action', label: 'P', config: { connectorId: 'crm', actionId: 'push', input: {} } }, + { id: 'push', type: 'connector_action', label: 'P', connectorConfig: { connectorId: 'crm', actionId: 'push', input: {} } }, { id: 'end', type: 'end', label: 'E' }, ], edges: [{ id: 'e1', source: 'start', target: 'push' }, { id: 'e2', source: 'push', target: 'end' }], @@ -977,7 +977,7 @@ describe('uncountable effects (#4354 follow-up)', () => { name: 'f', label: 'f', type: 'autolaunched', nodes: [ { id: 'start', type: 'start', label: 'S' }, - { id: 'push', type: 'connector_action', label: 'P', config: { connectorId: 'crm', actionId: 'push', input: {} } }, + { id: 'push', type: 'connector_action', label: 'P', connectorConfig: { connectorId: 'crm', actionId: 'push', input: {} } }, { id: 'end', type: 'end', label: 'E' }, ], edges: [{ id: 'e1', source: 'start', target: 'push' }, { id: 'e2', source: 'push', target: 'end' }], @@ -1003,7 +1003,7 @@ describe('uncountable effects (#4354 follow-up)', () => { name: 'child', label: 'child', type: 'autolaunched', nodes: [ { id: 'start', type: 'start', label: 'S' }, - { id: 'push', type: 'connector_action', label: 'P', config: { connectorId: 'crm', actionId: 'push', input: {} } }, + { id: 'push', type: 'connector_action', label: 'P', connectorConfig: { connectorId: 'crm', actionId: 'push', input: {} } }, { id: 'end', type: 'end', label: 'E' }, ], edges: [{ id: 'e1', source: 'start', target: 'push' }, { id: 'e2', source: 'push', target: 'end' }],