diff --git a/.changeset/notify-source-shape-conversion.md b/.changeset/notify-source-shape-conversion.md new file mode 100644 index 0000000000..91315f497e --- /dev/null +++ b/.changeset/notify-source-shape-conversion.md @@ -0,0 +1,36 @@ +--- +'@objectstack/spec': patch +'@objectstack/service-automation': patch +--- + +Graduate `notify`'s nested `source: { object, id }` into the conversion layer (#4045). + +The `notify` executor tolerated a second spelling of its click-through target with +a bare consumer-side fallback: + +```ts +const object = toStr(interpolate(cfg.sourceObject ?? src?.object, …)); +``` + +Its own doc comment named `sourceObject`/`sourceId` **canonical** (they mirror the +`sys_notification.source_object`/`source_id` columns), so the nested form was an +alias tolerated by exactly the mechanism Prime Directive #12 calls debt — and the +one alias on this executor that #3796 missed when it moved `to`/`subject`/`body`/ +`url` into `flow-node-notify-config-aliases`. + +It now graduates the same way `filters` → `filter` and `object` → `objectName` +did: the conversion lifts it onto the canonical pair at load — including the +`AutomationEngine.registerFlow` rehydration seam — and the executor's fallback is +deleted, so no consumer-side dialect tolerance survives and the alias is declared, +tested and retirable on schedule (it rides the existing entry's window, retiring +at 18). + +Unlike the four renames this is a **1→2 destructuring**, which the pair mechanism +cannot express, so it is a small custom transform. It mirrors the `??` precedence +exactly: a canonical key already present wins and its nested counterpart is left +shadowed, matching how a shadowed alias is treated elsewhere. `source` is dropped +once at least one part is lifted; a `source` that is not an object, or carries +neither key, is left untouched rather than silently deleted. + +No behaviour change for authors — both spellings keep working, and a +half-specified target is still dropped rather than emitting a dead deep-link. diff --git a/docs/protocol-upgrade-guide.md b/docs/protocol-upgrade-guide.md index 279d0aecc4..787b9f9443 100644 --- a/docs/protocol-upgrade-guide.md +++ b/docs/protocol-upgrade-guide.md @@ -142,7 +142,7 @@ On the wire contract it also retires the `/analytics/query` request ENVELOPE (#3 | `agent-tools-to-skills` | `agent.tools` | agent key 'tools' removed — declare capability in a skill (ADR-0064, #3894) | retired — `migrate meta` only | | `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) | 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-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/packages/services/service-automation/src/builtin/notify-node.test.ts b/packages/services/service-automation/src/builtin/notify-node.test.ts index 2e4c5980b6..9b935c7ee3 100644 --- a/packages/services/service-automation/src/builtin/notify-node.test.ts +++ b/packages/services/service-automation/src/builtin/notify-node.test.ts @@ -137,6 +137,11 @@ describe('notify (baseline node)', () => { }); }); + // #4045 — this now proves the CONVERSION, not executor tolerance. The + // executor reads only the canonical `sourceObject`/`sourceId`; the nested + // form reaches them because `registerFlow` applies + // `flow-node-notify-config-aliases`, which lifts it. Verified by disabling + // the lift: `source` comes back `undefined` and this test fails. it('accepts the nested source:{object,id} form and forwards actorId', async () => { engine.registerFlow('notify_flow', notifyFlow({ recipients: ['user_1'], diff --git a/packages/services/service-automation/src/builtin/notify-node.ts b/packages/services/service-automation/src/builtin/notify-node.ts index 3bf47d5b38..5517b37ad9 100644 --- a/packages/services/service-automation/src/builtin/notify-node.ts +++ b/packages/services/service-automation/src/builtin/notify-node.ts @@ -78,20 +78,26 @@ function toStr(value: unknown): string | undefined { /** * Resolve the click-through target record from the node config, if any. * - * Accepts the flat `sourceObject`/`sourceId` keys (canonical — mirrors the - * `sys_notification.source_object`/`source_id` columns) or the nested - * `source: { object, id }` form (mirrors the messaging `emit()` surface). A - * target is produced only when BOTH object and id resolve — a half-specified - * link is dropped so the inbox never renders a dead deep-link. + * Reads the canonical flat `sourceObject`/`sourceId` keys only — they mirror the + * `sys_notification.source_object`/`source_id` columns. A target is produced + * only when BOTH object and id resolve; a half-specified link is dropped so the + * inbox never renders a dead deep-link. + * + * The nested `source: { object, id }` form (the messaging `emit()` surface) used + * to be tolerated here by a bare `cfg.sourceObject ?? src?.object`. It has + * graduated into the ADR-0087 D2 conversion layer + * (`flow-node-notify-config-aliases`, #4045), which lifts it onto the flat pair + * at load — including the `registerFlow` rehydration seam — so no consumer-side + * fallback survives and the alias is declared, tested and retirable on schedule + * (Prime Directive #12). Same graduation path as `object` → `objectName` (#3796). */ function resolveSource( cfg: Record, variables: VariableMap, context: AutomationContext, ): { object: string; id: string } | undefined { - const src = (cfg.source ?? null) as { object?: unknown; id?: unknown } | null; - const object = toStr(interpolate(cfg.sourceObject ?? src?.object, variables, context)); - const id = toStr(interpolate(cfg.sourceId ?? src?.id, variables, context)); + const object = toStr(interpolate(cfg.sourceObject, variables, context)); + const id = toStr(interpolate(cfg.sourceId, variables, context)); return object && id ? { object, id } : undefined; } diff --git a/packages/spec/spec-changes.json b/packages/spec/spec-changes.json index 6d2827666c..738fe998e1 100644 --- a/packages/spec/spec-changes.json +++ b/packages/spec/spec-changes.json @@ -106,7 +106,7 @@ }, { "surface": "flow.node.notify.config", - "to": "notify flow-node config keys 'to' → 'recipients', 'subject' → 'title', 'body' → 'message', 'url' → 'actionUrl' (#3796)", + "to": "notify flow-node config keys 'to' → 'recipients', 'subject' → 'title', 'body' → 'message', 'url' → 'actionUrl' (#3796), and nested 'source: {object, id}' → 'sourceObject' / 'sourceId' (#4045)", "conversionId": "flow-node-notify-config-aliases", "toMajor": 17 }, @@ -454,7 +454,7 @@ }, { "surface": "flow.node.notify.config", - "to": "notify flow-node config keys 'to' → 'recipients', 'subject' → 'title', 'body' → 'message', 'url' → 'actionUrl' (#3796)", + "to": "notify flow-node config keys 'to' → 'recipients', 'subject' → 'title', 'body' → 'message', 'url' → 'actionUrl' (#3796), and nested 'source: {object, id}' → 'sourceObject' / 'sourceId' (#4045)", "conversionId": "flow-node-notify-config-aliases", "toMajor": 17 }, diff --git a/packages/spec/src/conversions/registry.ts b/packages/spec/src/conversions/registry.ts index 43a75ba664..9154b16968 100644 --- a/packages/spec/src/conversions/registry.ts +++ b/packages/spec/src/conversions/registry.ts @@ -959,12 +959,52 @@ const flowNodeCrudObjectAlias: MetadataConversion = { }; /** - * Notify flow-node config key aliases → canonical (protocol 17, #3796). + * Lift `notify`'s nested `config.source: { object, id }` onto the canonical flat + * `sourceObject` / `sourceId` keys (#4045). * - * The `notify` executor carried four open-coded `??` fallbacks that never went + * The fifth notify alias, and the only one that is not a 1:1 rename — it is a + * 1→2 destructuring, so {@link renameFlowConfigAliases}' pair mechanism cannot + * express it. Semantics mirror the `??` precedence the executor used to carry: + * a canonical key already present WINS and its nested counterpart is left + * shadowed, exactly as {@link renameConfigKey} treats a shadowed alias. + * + * `source` is dropped once at least one part was lifted — every part is by then + * either lifted or shadowed by a canonical key, so nothing observable is lost + * (the executor only ever read `.object` / `.id`). A `source` that is not a dict, + * or carries neither key, is left untouched rather than silently deleted. + */ +function liftNotifySourceShape(stack: Dict, emit: Emit): Dict { + return mapFlowNodes(stack, (node, path) => { + if (node.type !== 'notify') return node; + const config = node.config; + if (!isDict(config)) return node; + const source = config.source; + if (!isDict(source)) return node; + + const nextConfig: Dict = { ...config }; + let lifted = false; + for (const [from, to] of [['object', 'sourceObject'], ['id', 'sourceId']] as const) { + if (source[from] == null) continue; + if (nextConfig[to] != null) continue; // canonical already wins + nextConfig[to] = source[from]; + emit({ from: `source.${from}`, to, path: `${path}.config.${to}` }); + lifted = true; + } + if (!lifted) return node; + delete nextConfig.source; + return { ...node, config: nextConfig }; + }); +} + +/** + * Notify flow-node config key aliases → canonical (protocol 17, #3796 / #4045). + * + * The `notify` executor carried five open-coded `??` fallbacks that never went * through the deprecation shim — an author who wrote the email-idiom keys got * a flow that worked forever and was never steered to the canonical spelling. - * All four are pure key renames with unchanged values. + * Four are pure key renames with unchanged values; the fifth + * (`source: { object, id }`, #4045) is a destructuring handled by + * {@link liftNotifySourceShape}. * * `actionUrl` is the deliberate canonical of its pair (the executor's own * `configSchema` used to claim the opposite): the entire downstream chain @@ -980,9 +1020,10 @@ const flowNodeNotifyConfigAliases: MetadataConversion = { toMajor: 17, surface: 'flow.node.notify.config', summary: - "notify flow-node config keys 'to' → 'recipients', 'subject' → 'title', 'body' → 'message', 'url' → 'actionUrl' (#3796)", + "notify flow-node config keys 'to' → 'recipients', 'subject' → 'title', 'body' → 'message', 'url' → 'actionUrl' (#3796), " + + "and nested 'source: {object, id}' → 'sourceObject' / 'sourceId' (#4045)", apply(stack, emit) { - return renameFlowConfigAliases( + const renamed = renameFlowConfigAliases( stack, new Set(['notify']), [ @@ -993,6 +1034,7 @@ const flowNodeNotifyConfigAliases: MetadataConversion = { ], emit, ); + return liftNotifySourceShape(renamed, emit); }, fixture: { before: { @@ -1010,6 +1052,19 @@ const flowNodeNotifyConfigAliases: MetadataConversion = { body: 'You have been assigned "{record.title}".', url: '/task/{record.id}', channels: ['inbox'], + // #4045 — the nested click-through target, lifted to the flat pair. + source: { object: 'showcase_task', id: '{record.id}' }, + }, + }, + // A canonical `sourceObject` WINS: only the unshadowed `id` is + // lifted, and `source` is dropped since every part is accounted for. + { + id: 'n3', + type: 'notify', + config: { + recipients: ['{record.owner}'], + sourceObject: 'showcase_project', + source: { object: 'ignored', id: '{record.project}' }, }, }, ], @@ -1031,13 +1086,26 @@ const flowNodeNotifyConfigAliases: MetadataConversion = { message: 'You have been assigned "{record.title}".', actionUrl: '/task/{record.id}', channels: ['inbox'], + sourceObject: 'showcase_task', + sourceId: '{record.id}', + }, + }, + { + id: 'n3', + type: 'notify', + config: { + recipients: ['{record.owner}'], + sourceObject: 'showcase_project', + sourceId: '{record.project}', }, }, ], }, ], }, - expectedNotices: 4, + // 4 renames on n2 + `source.object`/`source.id` lifted on n2 + the single + // unshadowed `source.id` on n3 (its `source.object` is shadowed → no notice). + expectedNotices: 7, }, };