Skip to content

Commit 01e124d

Browse files
authored
fix(automation,spec): graduate notify's nested source into the conversion layer (#4045) (#4050)
The `notify` executor tolerated a second spelling of its click-through target with a bare `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 `source: { object, id }` form was an alias held up by exactly the mechanism Prime Directive #12 calls debt — and the one alias on this executor that #3796 missed. Graduated like `filters` → `filter` (#2645) and `object` → `objectName` (#3796): lifted onto the canonical pair at load, including the registerFlow rehydration seam, and the executor fallback deleted. Unlike the four renames this is a 1→2 destructuring, so it is a small custom transform on the existing entry (same window — retires at 18). It mirrors the old `??` precedence: a canonical key already present WINS and its nested counterpart is left shadowed; `source` is dropped once at least one part is lifted; a non-dict `source`, or one carrying neither key, is left untouched. The pre-existing nested-source executor test now proves the CONVERSION rather than executor tolerance — verified by disabling the lift, which makes it fail.
1 parent 3eb776a commit 01e124d

6 files changed

Lines changed: 132 additions & 17 deletions

File tree

Lines changed: 36 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,36 @@
1+
---
2+
'@objectstack/spec': patch
3+
'@objectstack/service-automation': patch
4+
---
5+
6+
Graduate `notify`'s nested `source: { object, id }` into the conversion layer (#4045).
7+
8+
The `notify` executor tolerated a second spelling of its click-through target with
9+
a bare consumer-side fallback:
10+
11+
```ts
12+
const object = toStr(interpolate(cfg.sourceObject ?? src?.object, …));
13+
```
14+
15+
Its own doc comment named `sourceObject`/`sourceId` **canonical** (they mirror the
16+
`sys_notification.source_object`/`source_id` columns), so the nested form was an
17+
alias tolerated by exactly the mechanism Prime Directive #12 calls debt — and the
18+
one alias on this executor that #3796 missed when it moved `to`/`subject`/`body`/
19+
`url` into `flow-node-notify-config-aliases`.
20+
21+
It now graduates the same way `filters``filter` and `object``objectName`
22+
did: the conversion lifts it onto the canonical pair at load — including the
23+
`AutomationEngine.registerFlow` rehydration seam — and the executor's fallback is
24+
deleted, so no consumer-side dialect tolerance survives and the alias is declared,
25+
tested and retirable on schedule (it rides the existing entry's window, retiring
26+
at 18).
27+
28+
Unlike the four renames this is a **1→2 destructuring**, which the pair mechanism
29+
cannot express, so it is a small custom transform. It mirrors the `??` precedence
30+
exactly: a canonical key already present wins and its nested counterpart is left
31+
shadowed, matching how a shadowed alias is treated elsewhere. `source` is dropped
32+
once at least one part is lifted; a `source` that is not an object, or carries
33+
neither key, is left untouched rather than silently deleted.
34+
35+
No behaviour change for authors — both spellings keep working, and a
36+
half-specified target is still dropped rather than emitting a dead deep-link.

docs/protocol-upgrade-guide.md

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -142,7 +142,7 @@ On the wire contract it also retires the `/analytics/query` request ENVELOPE (#3
142142
| `agent-tools-to-skills` | `agent.tools` | agent key 'tools' removed — declare capability in a skill (ADR-0064, #3894) | retired — `migrate meta` only |
143143
| `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 |
144144
| `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 |
145-
| `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 |
145+
| `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 |
146146
| `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 |
147147
| `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 |
148148
| `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

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -137,6 +137,11 @@ describe('notify (baseline node)', () => {
137137
});
138138
});
139139

140+
// #4045 — this now proves the CONVERSION, not executor tolerance. The
141+
// executor reads only the canonical `sourceObject`/`sourceId`; the nested
142+
// form reaches them because `registerFlow` applies
143+
// `flow-node-notify-config-aliases`, which lifts it. Verified by disabling
144+
// the lift: `source` comes back `undefined` and this test fails.
140145
it('accepts the nested source:{object,id} form and forwards actorId', async () => {
141146
engine.registerFlow('notify_flow', notifyFlow({
142147
recipients: ['user_1'],

packages/services/service-automation/src/builtin/notify-node.ts

Lines changed: 14 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -78,20 +78,26 @@ function toStr(value: unknown): string | undefined {
7878
/**
7979
* Resolve the click-through target record from the node config, if any.
8080
*
81-
* Accepts the flat `sourceObject`/`sourceId` keys (canonical — mirrors the
82-
* `sys_notification.source_object`/`source_id` columns) or the nested
83-
* `source: { object, id }` form (mirrors the messaging `emit()` surface). A
84-
* target is produced only when BOTH object and id resolve — a half-specified
85-
* link is dropped so the inbox never renders a dead deep-link.
81+
* Reads the canonical flat `sourceObject`/`sourceId` keys only — they mirror the
82+
* `sys_notification.source_object`/`source_id` columns. A target is produced
83+
* only when BOTH object and id resolve; a half-specified link is dropped so the
84+
* inbox never renders a dead deep-link.
85+
*
86+
* The nested `source: { object, id }` form (the messaging `emit()` surface) used
87+
* to be tolerated here by a bare `cfg.sourceObject ?? src?.object`. It has
88+
* graduated into the ADR-0087 D2 conversion layer
89+
* (`flow-node-notify-config-aliases`, #4045), which lifts it onto the flat pair
90+
* at load — including the `registerFlow` rehydration seam — so no consumer-side
91+
* fallback survives and the alias is declared, tested and retirable on schedule
92+
* (Prime Directive #12). Same graduation path as `object` → `objectName` (#3796).
8693
*/
8794
function resolveSource(
8895
cfg: Record<string, unknown>,
8996
variables: VariableMap,
9097
context: AutomationContext,
9198
): { object: string; id: string } | undefined {
92-
const src = (cfg.source ?? null) as { object?: unknown; id?: unknown } | null;
93-
const object = toStr(interpolate(cfg.sourceObject ?? src?.object, variables, context));
94-
const id = toStr(interpolate(cfg.sourceId ?? src?.id, variables, context));
99+
const object = toStr(interpolate(cfg.sourceObject, variables, context));
100+
const id = toStr(interpolate(cfg.sourceId, variables, context));
95101
return object && id ? { object, id } : undefined;
96102
}
97103

packages/spec/spec-changes.json

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -106,7 +106,7 @@
106106
},
107107
{
108108
"surface": "flow.node.notify.config",
109-
"to": "notify flow-node config keys 'to' → 'recipients', 'subject' → 'title', 'body' → 'message', 'url' → 'actionUrl' (#3796)",
109+
"to": "notify flow-node config keys 'to' → 'recipients', 'subject' → 'title', 'body' → 'message', 'url' → 'actionUrl' (#3796), and nested 'source: {object, id}' → 'sourceObject' / 'sourceId' (#4045)",
110110
"conversionId": "flow-node-notify-config-aliases",
111111
"toMajor": 17
112112
},
@@ -454,7 +454,7 @@
454454
},
455455
{
456456
"surface": "flow.node.notify.config",
457-
"to": "notify flow-node config keys 'to' → 'recipients', 'subject' → 'title', 'body' → 'message', 'url' → 'actionUrl' (#3796)",
457+
"to": "notify flow-node config keys 'to' → 'recipients', 'subject' → 'title', 'body' → 'message', 'url' → 'actionUrl' (#3796), and nested 'source: {object, id}' → 'sourceObject' / 'sourceId' (#4045)",
458458
"conversionId": "flow-node-notify-config-aliases",
459459
"toMajor": 17
460460
},

packages/spec/src/conversions/registry.ts

Lines changed: 74 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -959,12 +959,52 @@ const flowNodeCrudObjectAlias: MetadataConversion = {
959959
};
960960

961961
/**
962-
* Notify flow-node config key aliases → canonical (protocol 17, #3796).
962+
* Lift `notify`'s nested `config.source: { object, id }` onto the canonical flat
963+
* `sourceObject` / `sourceId` keys (#4045).
963964
*
964-
* The `notify` executor carried four open-coded `??` fallbacks that never went
965+
* The fifth notify alias, and the only one that is not a 1:1 rename — it is a
966+
* 1→2 destructuring, so {@link renameFlowConfigAliases}' pair mechanism cannot
967+
* express it. Semantics mirror the `??` precedence the executor used to carry:
968+
* a canonical key already present WINS and its nested counterpart is left
969+
* shadowed, exactly as {@link renameConfigKey} treats a shadowed alias.
970+
*
971+
* `source` is dropped once at least one part was lifted — every part is by then
972+
* either lifted or shadowed by a canonical key, so nothing observable is lost
973+
* (the executor only ever read `.object` / `.id`). A `source` that is not a dict,
974+
* or carries neither key, is left untouched rather than silently deleted.
975+
*/
976+
function liftNotifySourceShape(stack: Dict, emit: Emit): Dict {
977+
return mapFlowNodes(stack, (node, path) => {
978+
if (node.type !== 'notify') return node;
979+
const config = node.config;
980+
if (!isDict(config)) return node;
981+
const source = config.source;
982+
if (!isDict(source)) return node;
983+
984+
const nextConfig: Dict = { ...config };
985+
let lifted = false;
986+
for (const [from, to] of [['object', 'sourceObject'], ['id', 'sourceId']] as const) {
987+
if (source[from] == null) continue;
988+
if (nextConfig[to] != null) continue; // canonical already wins
989+
nextConfig[to] = source[from];
990+
emit({ from: `source.${from}`, to, path: `${path}.config.${to}` });
991+
lifted = true;
992+
}
993+
if (!lifted) return node;
994+
delete nextConfig.source;
995+
return { ...node, config: nextConfig };
996+
});
997+
}
998+
999+
/**
1000+
* Notify flow-node config key aliases → canonical (protocol 17, #3796 / #4045).
1001+
*
1002+
* The `notify` executor carried five open-coded `??` fallbacks that never went
9651003
* through the deprecation shim — an author who wrote the email-idiom keys got
9661004
* a flow that worked forever and was never steered to the canonical spelling.
967-
* All four are pure key renames with unchanged values.
1005+
* Four are pure key renames with unchanged values; the fifth
1006+
* (`source: { object, id }`, #4045) is a destructuring handled by
1007+
* {@link liftNotifySourceShape}.
9681008
*
9691009
* `actionUrl` is the deliberate canonical of its pair (the executor's own
9701010
* `configSchema` used to claim the opposite): the entire downstream chain
@@ -980,9 +1020,10 @@ const flowNodeNotifyConfigAliases: MetadataConversion = {
9801020
toMajor: 17,
9811021
surface: 'flow.node.notify.config',
9821022
summary:
983-
"notify flow-node config keys 'to' → 'recipients', 'subject' → 'title', 'body' → 'message', 'url' → 'actionUrl' (#3796)",
1023+
"notify flow-node config keys 'to' → 'recipients', 'subject' → 'title', 'body' → 'message', 'url' → 'actionUrl' (#3796), " +
1024+
"and nested 'source: {object, id}' → 'sourceObject' / 'sourceId' (#4045)",
9841025
apply(stack, emit) {
985-
return renameFlowConfigAliases(
1026+
const renamed = renameFlowConfigAliases(
9861027
stack,
9871028
new Set(['notify']),
9881029
[
@@ -993,6 +1034,7 @@ const flowNodeNotifyConfigAliases: MetadataConversion = {
9931034
],
9941035
emit,
9951036
);
1037+
return liftNotifySourceShape(renamed, emit);
9961038
},
9971039
fixture: {
9981040
before: {
@@ -1010,6 +1052,19 @@ const flowNodeNotifyConfigAliases: MetadataConversion = {
10101052
body: 'You have been assigned "{record.title}".',
10111053
url: '/task/{record.id}',
10121054
channels: ['inbox'],
1055+
// #4045 — the nested click-through target, lifted to the flat pair.
1056+
source: { object: 'showcase_task', id: '{record.id}' },
1057+
},
1058+
},
1059+
// A canonical `sourceObject` WINS: only the unshadowed `id` is
1060+
// lifted, and `source` is dropped since every part is accounted for.
1061+
{
1062+
id: 'n3',
1063+
type: 'notify',
1064+
config: {
1065+
recipients: ['{record.owner}'],
1066+
sourceObject: 'showcase_project',
1067+
source: { object: 'ignored', id: '{record.project}' },
10131068
},
10141069
},
10151070
],
@@ -1031,13 +1086,26 @@ const flowNodeNotifyConfigAliases: MetadataConversion = {
10311086
message: 'You have been assigned "{record.title}".',
10321087
actionUrl: '/task/{record.id}',
10331088
channels: ['inbox'],
1089+
sourceObject: 'showcase_task',
1090+
sourceId: '{record.id}',
1091+
},
1092+
},
1093+
{
1094+
id: 'n3',
1095+
type: 'notify',
1096+
config: {
1097+
recipients: ['{record.owner}'],
1098+
sourceObject: 'showcase_project',
1099+
sourceId: '{record.project}',
10341100
},
10351101
},
10361102
],
10371103
},
10381104
],
10391105
},
1040-
expectedNotices: 4,
1106+
// 4 renames on n2 + `source.object`/`source.id` lifted on n2 + the single
1107+
// unshadowed `source.id` on n3 (its `source.object` is shadowed → no notice).
1108+
expectedNotices: 7,
10411109
},
10421110
};
10431111

0 commit comments

Comments
 (0)