fix(automation,spec): graduate notify's nested source into the conversion layer (#4045)

.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.

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 |

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'],

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<string, unknown>,
     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;
 }
 

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
         },

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,
   },
 };