diff --git a/.changeset/wait-loose-config-graduation.md b/.changeset/wait-loose-config-graduation.md new file mode 100644 index 0000000000..b3cc6b16b2 --- /dev/null +++ b/.changeset/wait-loose-config-graduation.md @@ -0,0 +1,46 @@ +--- +'@objectstack/spec': patch +'@objectstack/service-automation': patch +--- + +The `wait` executor reads its declared contract only; the loose `config` back door graduates into the conversion layer (#4045). + +`wait` keeps its contract in `waitEventConfig` — a declared, `.describe()`-annotated +block on `FlowNodeSchema` that is in the authorable-field list, reaches the generated +reference, and is what the showcase actually authors. Its descriptor publishes no +`configSchema`, which is by design rather than the gap it first looks like. + +The executor nevertheless also read six loose `config` keys behind `wec.X ?? loose.X`, +two of them (`duration`, `signal`) spellings the spec never declared anywhere. That is +the `notify.source` shape #4050 retired: a second de-facto contract announced only by a +code comment, so an author who wrote it got a flow that worked forever and was never +steered to the declared spelling (PD #12). Not hypothetical: the showcase's own +`wait_revision` node authored it (`config: { eventType: 'signal', signalName: … }`) and +moves to the declared block here. + +- New ADR-0087 D2 conversion `flow-node-wait-event-config-lift` lifts + `config.{eventType,timerDuration,duration,timeoutMs,signalName,signal}` onto the + declared `waitEventConfig` block, in the executor's own `??` precedence — a declared + value wins and its loose counterpart is left shadowed, exactly as `renameConfigKey` + treats a shadowed alias. +- `eventType` is stamped `'timer'` whenever the lift would otherwise leave the block + without one. This is load-bearing: the loader parses the **converted** flow + (`applyConversionsToFlow` → `FlowSchema.parse`) and `waitEventConfig.eventType` is + required once the block exists — so a stored flow carrying only + `config: { duration: 'PT1M' }` would have gone from working to failing to load. + `'timer'` is the exact default the executor applied to that shape. +- The executor's six `?? loose.*` fallbacks are deleted. The surviving `?? 'timer'` is + not one: `waitEventConfig` is itself optional, and a wait node without one is a valid + timer wait. + +Verified at the real seam: the new executor tests author the legacy shape and go through +`registerFlow`, which is what applies the conversion, so they prove the graduation +end-to-end on a legacy source rather than only that the executor stopped looking. A +negative control pins the `eventType` default — deleting it from the converted output +makes `FlowSchema.parse` throw. + +Two things this deliberately does **not** change, filed as #4158 rather than fixed in +passing: `waitEventConfig.timeoutMs` is declared as a timeout guard but read as a timer +duration, and `waitEventConfig.onTimeout` has zero readers anywhere — so `wait` has no +timeout implementation at all, while the showcase authors `onTimeout: 'continue'`. +Implementing or retracting that is a behaviour change, not a contract cleanup. diff --git a/docs/protocol-upgrade-guide.md b/docs/protocol-upgrade-guide.md index 44aa216d0b..f514c7b278 100644 --- a/docs/protocol-upgrade-guide.md +++ b/docs/protocol-upgrade-guide.md @@ -124,6 +124,8 @@ Finally it removes agent `tools` (#3894): the legacy inline `{type,name,descript Beyond those spec-surface removals, it graduates the seven flow-node config key aliases the executors still tolerated (#3796): the CRUD nodes' `object` (use `objectName`) — the last tenant of the `readAliasedConfig` executor shim, which is deleted with it — plus the six open-coded fallbacks that never went through that shim: notify `to`/`subject`/`body`/`url` (use `recipients`/`title`/`message`/`actionUrl`) and script `functionName`/`input` (use `function`/`inputs`). All are pure key renames with unchanged values and replay losslessly. Like the sharing-rule access level above they keep a load-path acceptance window: none carried a prior deprecation warning, and `FlowNodeSchema.config` is an unconstrained record, so no schema tombstone can reject them — the conversion layer is the only seam that can declare, convert, and retire them. +The same graduation covers `wait`, whose fallback was not a config-to-config rename (#4045). `wait` keeps its contract in the declared `waitEventConfig` block, not in `config` at all — yet the executor also read six loose `config` keys, two of them (`duration`, `signal`) spellings the spec never declared. The conversion lifts them onto the declared block in the executor's own `??` precedence, so a value already declared wins and its loose counterpart is left shadowed. One wrinkle makes this a rewrite rather than a delete: `waitEventConfig.eventType` is required once the block exists, and the loader parses the CONVERTED flow — so a source carrying only `config: { duration }` is stamped with `eventType: 'timer'`, the exact default the executor applied to that shape. Behaviour-preserving in both directions. + And it removes the RLS-policy key `priority` (#3896 security audit): promised "conflict resolution" that cannot exist, because applicable policies OR-combine (most permissive wins) — there is never a conflict to order, and nothing ever read the key (call graph closed across the collection site, the projection round-trip and the compiler). A pure lossless delete: outcomes are identical with or without it; the schema tombstones the key with the same prescription. The same close-out retires the four inert tool authoring keys (`category`, `permissions`, `active`, `builtIn`): none is part of AIToolDefinition and no execution path read them. Two were misleading in the dangerous direction — `permissions` promised an invocation gate nothing enforced, and `active: false` read as "withdrawn" while the tool kept reaching the LLM tool set. Lossless deletes; the strict ToolSchema rejects each with its prescription. @@ -146,6 +148,7 @@ The close-out sweep finishes the enforce-or-remove worklist across the remaining | `sharing-rule-access-level-full-to-edit` | `sharingRule.accessLevel` | sharing-rule accessLevel 'full' → 'edit' (#3865 — `full` never granted more than `edit`) | live — protocol 17 loader accepts the old shape | | `flow-node-crud-object-alias` | `flow.node.config.objectName` | CRUD flow-node config key 'object' → 'objectName' (#3796 — `readAliasedConfig` shim graduation) | live — protocol 17 loader accepts the old shape | | `flow-node-notify-config-aliases` | `flow.node.notify.config` | notify flow-node config keys 'to' → 'recipients', 'subject' → 'title', 'body' → 'message', 'url' → 'actionUrl' (#3796), and nested 'source: {object, id}' → 'sourceObject' / 'sourceId' (#4045) | live — protocol 17 loader accepts the old shape | +| `flow-node-wait-event-config-lift` | `flow.node.wait.waitEventConfig` | wait flow-node loose config keys → the declared `waitEventConfig` block: 'eventType', 'timerDuration'/'duration' → 'timerDuration', 'signalName'/'signal' → 'signalName', 'timeoutMs' (#4045) | live — protocol 17 loader accepts the old shape | | `flow-node-script-config-aliases` | `flow.node.script.config` | script flow-node config keys 'functionName' → 'function', 'input' → 'inputs' (#3796) | live — protocol 17 loader accepts the old shape | | `permission-rls-priority-removed` | `permission.rowLevelSecurity.priority` | RLS-policy key 'priority' removed (#3896 audit — policies OR-combine, so the promised conflict-resolution semantics cannot exist; dropping it changes no outcome) | retired — `migrate meta` only | | `tool-inert-authoring-keys-removed` | `tool.category / tool.permissions / tool.active / tool.builtIn` | tool keys 'category'/'permissions'/'active'/'builtIn' removed (#3896 close-out — authorable and inert; permissions gated nothing, active:false withdrew nothing) | retired — `migrate meta` only | diff --git a/examples/app-showcase/src/automation/flows/index.ts b/examples/app-showcase/src/automation/flows/index.ts index 48c9965bea..9f9829a4e1 100644 --- a/examples/app-showcase/src/automation/flows/index.ts +++ b/examples/app-showcase/src/automation/flows/index.ts @@ -206,7 +206,10 @@ export const BudgetApprovalFlow = defineFlow({ id: 'wait_revision', type: 'wait', label: 'Awaiting Revision', - config: { eventType: 'signal', signalName: 'budget_revision' }, + // `waitEventConfig`, not a loose `config` — the latter is the undeclared + // back door retired in #4045. The conversion layer still rewrites it at + // load, but the showcase should demonstrate the declared spelling. + waitEventConfig: { eventType: 'signal', signalName: 'budget_revision' }, }, { id: 'needs_exec', 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 add4490824..2a4b5e5496 100644 --- a/packages/services/service-automation/src/builtin/wait-node.test.ts +++ b/packages/services/service-automation/src/builtin/wait-node.test.ts @@ -140,6 +140,101 @@ describe('wait node executor', () => { }); }); +/** + * The loose `config.*` back door the executor used to read alongside + * `waitEventConfig` graduated into the ADR-0087 D2 conversion layer + * (`flow-node-wait-event-config-lift`, #4045), so the executor now reads the + * declared block only (PD #12). + * + * 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`) + * are ones the spec never declared; they existed only as the tail of the `??` + * chains this change deleted. + */ +describe('wait config graduation — legacy loose `config` still works via the conversion (#4045)', () => { + let engine: AutomationEngine; + let ran: string[]; + + /** The same flow, but authored the legacy way: event keys loose under `config`. */ + const looseWaitFlow = (config: Record) => ({ + ...waitFlow({ eventType: 'timer' }), + nodes: [ + { id: 'start', type: 'start', label: 'Start' }, + { id: 'pause', type: 'wait', label: 'Wait', config }, + { id: 'after', type: 'mark', label: 'After' }, + { id: 'end', type: 'end', label: 'End' }, + ], + }); + + beforeEach(() => { + engine = new AutomationEngine(silentLogger()); + ran = []; + engine.registerNodeExecutor(markerExecutor(ran)); + }); + + it('schedules the same timer from a loose `config.duration` (undeclared spelling, no eventType)', async () => { + const { ctx, scheduled } = fakeJobCtx(); + registerWaitNode(engine, ctx); + // No eventType anywhere: the conversion stamps the executor's own 'timer' + // default, without which the converted flow would not even parse. + engine.registerFlow('wait_flow', looseWaitFlow({ duration: 'PT2H' })); + + const before = Date.now(); + const paused = await engine.execute('wait_flow'); + expect(paused.status).toBe('paused'); + expect(ran).toEqual([]); + + expect(scheduled).toHaveLength(1); + const at = new Date(scheduled[0].schedule.at!).getTime(); + expect(at).toBeGreaterThanOrEqual(before + 7_200_000 - 1000); + expect(at).toBeLessThanOrEqual(Date.now() + 7_200_000 + 1000); + }); + + it('suspends on a loose `config.signal` (undeclared spelling) as its declared counterpart', async () => { + registerWaitNode(engine, ctxNoJob()); + engine.registerFlow('wait_flow', looseWaitFlow({ eventType: 'signal', signal: 'contract.renewed' })); + + const paused = await engine.execute('wait_flow'); + expect(paused.status).toBe('paused'); + expect(engine.listSuspendedRuns()[0]).toMatchObject({ nodeId: 'pause', correlation: 'contract.renewed' }); + + const resumed = await engine.resume(paused.runId!); + expect(resumed.success).toBe(true); + 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('reads the declared value, not its loose counterpart, when both are present', async () => { + const { ctx, scheduled } = fakeJobCtx(); + registerWaitNode(engine, ctx); + engine.registerFlow('wait_flow', { + ...waitFlow({ eventType: 'timer' }), + nodes: [ + { id: 'start', type: 'start', label: 'Start' }, + { + id: 'pause', + type: 'wait', + label: 'Wait', + waitEventConfig: { eventType: 'timer', timerDuration: 'PT1H' }, + config: { duration: 'PT9H' }, + }, + { id: 'after', type: 'mark', label: 'After' }, + { id: 'end', type: 'end', label: 'End' }, + ], + }); + + const before = Date.now(); + await engine.execute('wait_flow'); + // 1h (declared) — not 9h (loose). Same precedence the deleted `??` had. + const at = new Date(scheduled[0].schedule.at!).getTime(); + expect(at).toBeLessThanOrEqual(before + 3_600_000 + 1000); + }); +}); + describe('rearmSuspendedWaitTimers (cold-boot timer re-arm)', () => { /** Boot a fresh engine wired to `store` with the wait flow registered — one "process". */ function bootEngine(store: InMemorySuspendedRunStore, ctx: any, waitConfig: Record) { diff --git a/packages/services/service-automation/src/builtin/wait-node.ts b/packages/services/service-automation/src/builtin/wait-node.ts index 47ecb29822..86d202c2e2 100644 --- a/packages/services/service-automation/src/builtin/wait-node.ts +++ b/packages/services/service-automation/src/builtin/wait-node.ts @@ -49,18 +49,29 @@ export function registerWaitNode(engine: AutomationEngine, ctx: PluginContext): isAsync: true, }), async execute(node, variables, _context) { - // Prefer the spec-structured `waitEventConfig` block; fall back to a loose - // `config` for hand-authored flows that put the same keys under config. - const loose = (node.config ?? {}) as Record; + // `waitEventConfig` is the whole contract (`FlowNodeSchema`, flow.zod.ts). + // 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). + // + // The `?? 'timer'` below is NOT such a fallback: `waitEventConfig` is + // itself optional, and a wait node without one is a valid timer wait. const wec = (node.waitEventConfig ?? {}) as Record; - const eventType = String(wec.eventType ?? loose.eventType ?? 'timer'); + const eventType = String(wec.eventType ?? 'timer'); const runId = variables.get('$runId'); if (eventType === 'timer') { + // `timeoutMs` doubling as a duration is pre-existing behaviour, kept + // deliberately — the declared meaning is a timeout guard, and `wait` has + // no timeout implementation at all (`onTimeout` has zero readers). That + // gap is #4158; changing it here would be a behaviour change riding on a + // contract cleanup. const durationMs = - parseIsoDuration(wec.timerDuration ?? loose.timerDuration ?? loose.duration) ?? - (typeof wec.timeoutMs === 'number' ? wec.timeoutMs : undefined) ?? - (typeof loose.timeoutMs === 'number' ? (loose.timeoutMs as number) : undefined); + parseIsoDuration(wec.timerDuration) ?? + (typeof wec.timeoutMs === 'number' ? wec.timeoutMs : undefined); // Persist the wake deadline as node output: the engine writes output // to variables (`.waitUntil`) *before* snapshotting the @@ -105,7 +116,7 @@ export function registerWaitNode(engine: AutomationEngine, ctx: PluginContext): // signal / webhook / manual / condition — suspend; an external producer // resumes the run when the named event arrives. - const signal = String(wec.signalName ?? loose.signalName ?? loose.signal ?? `wait:${node.id}`); + const signal = String(wec.signalName ?? `wait:${node.id}`); return { success: true, suspend: true, correlation: signal }; }, }); diff --git a/packages/spec/spec-changes.json b/packages/spec/spec-changes.json index 18e5114e26..7c6c49b736 100644 --- a/packages/spec/spec-changes.json +++ b/packages/spec/spec-changes.json @@ -104,6 +104,12 @@ "conversionId": "flow-node-notify-config-aliases", "toMajor": 17 }, + { + "surface": "flow.node.wait.waitEventConfig", + "to": "wait flow-node loose config keys → the declared `waitEventConfig` block: 'eventType', 'timerDuration'/'duration' → 'timerDuration', 'signalName'/'signal' → 'signalName', 'timeoutMs' (#4045)", + "conversionId": "flow-node-wait-event-config-lift", + "toMajor": 17 + }, { "surface": "flow.node.script.config", "to": "script flow-node config keys 'functionName' → 'function', 'input' → 'inputs' (#3796)", @@ -495,6 +501,12 @@ "conversionId": "flow-node-notify-config-aliases", "toMajor": 17 }, + { + "surface": "flow.node.wait.waitEventConfig", + "to": "wait flow-node loose config keys → the declared `waitEventConfig` block: 'eventType', 'timerDuration'/'duration' → 'timerDuration', 'signalName'/'signal' → 'signalName', 'timeoutMs' (#4045)", + "conversionId": "flow-node-wait-event-config-lift", + "toMajor": 17 + }, { "surface": "flow.node.script.config", "to": "script flow-node config keys 'functionName' → 'function', 'input' → 'inputs' (#3796)", diff --git a/packages/spec/src/conversions/conversions.test.ts b/packages/spec/src/conversions/conversions.test.ts index 06b5cfecf8..987606fd38 100644 --- a/packages/spec/src/conversions/conversions.test.ts +++ b/packages/spec/src/conversions/conversions.test.ts @@ -2,6 +2,7 @@ import { describe, expect, it } from 'vitest'; +import { FlowSchema } from '../automation/flow.zod.js'; import { normalizeStackInput } from '../shared/metadata-collection.zod.js'; import { applyConversions, collectConversionNotices } from './apply.js'; import { ALL_CONVERSIONS, CONVERSIONS_BY_MAJOR } from './registry.js'; @@ -230,4 +231,114 @@ describe('conversion layer (ADR-0087 D2)', () => { expect(flows[0].nodes[0].type).toBe('http'); // converted }); }); + + describe('flow-node-wait-event-config-lift (PD #12 retirement, #4045)', () => { + /** + * One `wait` node in a flow that `FlowSchema` can actually parse — `label` is + * required on both the flow and every node, which the conversion fixtures + * (shape-only, never parsed) omit. + */ + const waitFlow = (node: Record) => ({ + flows: [ + { + name: 'settle', + label: 'Settle', + type: 'autolaunched', + edges: [], + nodes: [ + { id: 'n1', type: 'start', label: 'Start' }, + { id: 'w', type: 'wait', label: 'Wait', ...node }, + ], + }, + ], + }); + const waitNodeOf = (stack: Record) => (stack.flows as any[])[0].nodes[1]; + + it('lifts the undeclared `duration` / `signal` spellings onto the declared block', () => { + const { stack, notices } = collectConversionNotices(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' } })); + 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( + waitFlow({ + waitEventConfig: { eventType: 'timer', timerDuration: 'PT5M' }, + config: { duration: 'PT9M', timeoutMs: 60_000 }, + }), + ); + expect(waitNodeOf(stack).waitEventConfig).toEqual({ + eventType: 'timer', + timerDuration: 'PT5M', + timeoutMs: 60_000, + }); + // The shadowed alias is not deleted — same treatment `renameConfigKey` gives one. + expect(waitNodeOf(stack).config).toEqual({ duration: 'PT9M' }); + expect(notices).toHaveLength(1); + }); + + it('prefers the declared spelling over the undeclared one among loose keys', () => { + const { stack } = collectConversionNotices( + 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' }); + }); + + /** + * The exact shape the showcase's `wait_revision` node carried before this + * change — the declared spelling sitting in the undeclared LOCATION, which is + * 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' } }), + ); + expect(waitNodeOf(stack).waitEventConfig).toEqual({ + eventType: 'signal', + signalName: 'budget_revision', + }); + expect(waitNodeOf(stack).config).toEqual({}); + expect(notices).toHaveLength(2); + expect(() => FlowSchema.parse((stack.flows as any[])[0])).not.toThrow(); + }); + + 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)); + 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({ + flows: [{ name: 'f', nodes: [{ id: 'a', type: 'custom', config: { duration: 'PT1M' } }] }], + }); + expect((stack.flows as any[])[0].nodes[0].config).toEqual({ duration: 'PT1M' }); + expect(notices).toHaveLength(0); + }); + + /** + * The `eventType: 'timer'` default is load-bearing, not tidiness. The loader + * parses the CONVERTED flow, and `waitEventConfig.eventType` is required once + * the block exists — so this asserts both directions: the shape the + * conversion produces loads, and the shape it would have produced without the + * 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 converted = (stack.flows as any[])[0]; + expect(() => FlowSchema.parse(converted)).not.toThrow(); + + const withoutDefault = structuredClone(converted); + delete withoutDefault.nodes[1].waitEventConfig.eventType; + expect(() => FlowSchema.parse(withoutDefault)).toThrow(); + }); + }); }); diff --git a/packages/spec/src/conversions/registry.ts b/packages/spec/src/conversions/registry.ts index 44a1a32767..59c0771a2f 100644 --- a/packages/spec/src/conversions/registry.ts +++ b/packages/spec/src/conversions/registry.ts @@ -1087,6 +1087,152 @@ 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. + */ +const WAIT_EVENT_CONFIG_LIFTS: ReadonlyArray = [ + ['eventType', ['eventType']], + ['timerDuration', ['timerDuration', 'duration']], + ['signalName', ['signalName', 'signal']], + ['timeoutMs', ['timeoutMs']], +]; + +/** + * Lift `wait`'s loose `config.*` event keys onto the declared `waitEventConfig` + * sibling (#4045). + * + * The only conversion here whose destination is **not** another `config` key. + * `wait`'s contract does not live in `config` at all — it is + * `FlowNodeSchema.waitEventConfig` (`flow.zod.ts`), a fully `.describe()`-annotated + * block that is in the authorable-field list, reaches the generated reference, + * and is what the showcase actually authors. Its descriptor therefore publishes + * no `configSchema`, which is by design and not the gap it first looks like. + * + * The executor nevertheless carried `wec.X ?? loose.X` for six `config` keys — + * a second, undeclared de-facto contract of exactly the `notify.source` shape + * (PD #12), announced only by the comment "for hand-authored flows that put the + * same keys under config". + * + * The showcase's `wait_revision` node authored exactly that shape + * (`config: { eventType: 'signal', signalName: 'budget_revision' }`) until this + * change moved it to the declared block — so the back door was not hypothetical, + * and the example that demonstrates `wait` was itself on the retiring spelling. + * + * Precedence mirrors those `??` chains, so the rewrite is behaviour-preserving: + * a value already on `waitEventConfig` WINS and its loose counterpart is left + * shadowed (as {@link renameConfigKey} treats a shadowed alias), and among loose + * candidates the first one present decides. + * + * `eventType` is defaulted to `'timer'` whenever lifting would otherwise leave + * the block without one. That is load-bearing, not tidiness: the loader parses + * the CONVERTED flow (`applyConversionsToFlow` → `FlowSchema.parse`), and + * `waitEventConfig.eventType` is **required** once the block exists — so a + * stored flow carrying only `config: { duration: 'PT1M' }` would go from working + * to failing to load. `'timer'` is the exact default the executor applied to + * that shape. + */ +function liftWaitEventConfig(stack: Dict, emit: Emit): Dict { + return mapFlowNodes(stack, (node, path) => { + if (node.type !== 'wait') return node; + const config = node.config; + if (!isDict(config)) return node; + + const wec: Dict = isDict(node.waitEventConfig) ? { ...node.waitEventConfig } : {}; + const nextConfig: Dict = { ...config }; + let lifted = false; + + for (const [target, candidates] of WAIT_EVENT_CONFIG_LIFTS) { + for (const from of candidates) { + if (nextConfig[from] == null) continue; + if (wec[target] == null) { + wec[target] = nextConfig[from]; + delete nextConfig[from]; + emit({ from: `config.${from}`, to: `waitEventConfig.${target}`, path: `${path}.waitEventConfig.${target}` }); + lifted = true; + } + // The first candidate PRESENT decides, lifted or shadowed — the `??` + // chain never looked past it either. + break; + } + } + + if (!lifted) return node; + if (wec.eventType == null) wec.eventType = 'timer'; + return { ...node, config: nextConfig, waitEventConfig: wec }; + }); +} + +/** + * Wait flow-node loose `config` keys → the declared `waitEventConfig` sibling + * (protocol 17, #4045). See {@link liftWaitEventConfig} for the precedence rules + * and why `eventType` is defaulted. **Live window**; retires at 18. + */ +const flowNodeWaitEventConfigLift: MetadataConversion = { + id: 'flow-node-wait-event-config-lift', + toMajor: 17, + surface: 'flow.node.wait.waitEventConfig', + summary: + "wait flow-node loose config keys → the declared `waitEventConfig` block: 'eventType', " + + "'timerDuration'/'duration' → 'timerDuration', 'signalName'/'signal' → 'signalName', 'timeoutMs' (#4045)", + apply(stack, emit) { + return liftWaitEventConfig(stack, emit); + }, + fixture: { + before: { + flows: [ + { + name: 'order_settlement', + nodes: [ + { id: 'n1', type: 'start' }, + // Loose-only, and via the UNDECLARED `duration` spelling with no + // eventType anywhere — the shape that would stop loading without + // the `'timer'` default. + { id: 'n2', type: 'wait', config: { duration: 'PT1M' } }, + // Loose `signal` alongside an explicit eventType: both lift. + { id: 'n3', type: 'wait', config: { eventType: 'signal', signal: 'order_paid' } }, + // Partially shadowed: `timerDuration` is already declared, so the + // loose `duration` stays put untouched; only `timeoutMs` lifts. + { + id: 'n4', + type: 'wait', + waitEventConfig: { eventType: 'timer', timerDuration: 'PT5M' }, + config: { duration: 'PT9M', timeoutMs: 60000 }, + }, + ], + }, + ], + }, + after: { + flows: [ + { + name: 'order_settlement', + nodes: [ + { id: 'n1', type: 'start' }, + { id: 'n2', type: 'wait', config: {}, waitEventConfig: { timerDuration: 'PT1M', eventType: 'timer' } }, + { id: 'n3', type: 'wait', config: {}, waitEventConfig: { eventType: 'signal', signalName: 'order_paid' } }, + { + id: 'n4', + type: 'wait', + waitEventConfig: { eventType: 'timer', timerDuration: 'PT5M', timeoutMs: 60000 }, + config: { duration: 'PT9M' }, + }, + ], + }, + ], + }, + // n2: `duration` → `timerDuration`. n3: `eventType` + `signal` → `signalName`. + // n4: only `timeoutMs` (its `duration` is shadowed by a declared + // `timerDuration` → no notice, and the key is left in place). + expectedNotices: 4, + }, +}; + /** * Script flow-node config key aliases → canonical (protocol 17, #3796). * @@ -1720,6 +1866,7 @@ export const CONVERSIONS_BY_MAJOR: Readonly