Skip to content

Commit 947d4f9

Browse files
os-zhuangclaude
andauthored
fix(spec): ADR-0087 别名转换按值处置被遮蔽的旧拼法(等值删除、异值双留) (#4923) (#5731)
* fix(spec): resolve a shadowed ADR-0087 alias by VALUE, not by always keeping it (#4923) `renameKey` / `renameConfigKey` did nothing whenever the canonical key was already present, so every D2 rename left the retired spelling sitting in the converted metadata — the conversion had not finished converting. Invisible while the flow-node config contracts were `.strip`; an execute-time guard refusal once #4001 batch 9 made them strict. Per the maintainer ruling recorded on the issue, the case now splits on whether the two values disagree: - equal (structural equality) -> the alias carries nothing the canonical key does not, so it is deleted and the usual notice fires. Lossless hygiene, inside the D2 contract, and idempotent in both shape and notices. - different -> both keys survive and nothing is emitted. Two spellings with two values is genuine author ambiguity; picking one would edit a config the customer never agreed to. The surviving pair is what lets the strict gates refuse with a prescription naming BOTH keys. `renameConfigKey` now delegates to `renameKey` so the rule has one definition, and the structural-equality predicate is the one the composer already used for "same value composes fine" (#5005), extracted to `shared/deep-equal.ts` so the two surfaces cannot disagree about what "the same" means. `liftNotifySourceShape` follows the same rule: a nested part repeating its flat counterpart is redundant (and `source` is dropped), a part that disagrees leaves the node entirely untouched so `source` reaches the strict contract. The `wait` node's loose-key lift is deliberately NOT covered and says so — it moves keys between locations rather than resolving two spellings of one slot. Batch-9 guidance reworded: a surviving twin is no longer "dead", it holds a value the canonical key does not, so each prescription names both keys and asks for a decision. Also adopts `mapPageComponents` (#5509) for protocol-15 `page-component-visibility-to-visibleWhen` per #5511 — its existing fixture is unchanged and stays green, which is the equivalence acceptance. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_018fxLGQdatPbBUvCgiVxg6D * fix(spec): guard the new equal-value branch against a self-rename, and finish the guidance sweep (#4923) Follow-up on the same ruling. `renameKey(dict, k, k)` would have reached the new equal-value branch, compared the value against itself, called it a redundant twin and deleted the only copy. No registry pair is written that way today (all 40 literal pairs scanned), so this is a guard rather than a fix: one added later now converts nothing instead of erasing data. Pinned by a test on both `renameKey` and `renameConfigKey`. Also completes the batch-9 wording sweep the ruling asked for: the notify guidance test still asserted the old "dead twin" reading, and the `source` prescription said DISAGREES where the rest of the family says DIFFERENT. Both now speak one vocabulary, and the test additionally requires each prescription to say the two values differ — not just that one of them should be deleted. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_018fxLGQdatPbBUvCgiVxg6D --------- Co-authored-by: Claude <noreply@anthropic.com>
1 parent 488b66c commit 947d4f9

12 files changed

Lines changed: 573 additions & 172 deletions
Lines changed: 51 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,51 @@
1+
---
2+
"@objectstack/spec": patch
3+
---
4+
5+
fix(spec): an ADR-0087 rename resolves a shadowed alias by VALUE, instead of always leaving it behind (#4923)
6+
7+
Every ADR-0087 D2 conversion that renames a key (`renameKey` /
8+
`renameConfigKey`, so `object` → `objectName`, `filters` → `filter`,
9+
`flow` → `flowName`, the four `notify` aliases, `description` → `subtitle`,
10+
the datasource driver-config aliases, …) used to do **nothing at all** when the
11+
canonical key was already present. The retired spelling then stayed in the
12+
converted metadata forever — the conversion had not finished converting.
13+
14+
That was invisible while flow-node config contracts were `.strip`: the dead key
15+
was silently dropped at the execute-time parse and the node ran. Once #4001 批 9
16+
made those contracts strict it stopped being invisible — a stored `subflow`
17+
carrying `{ flowName, flow }` loads today and would be refused at execute time
18+
as a guard failure (not routable through a `fault` edge).
19+
20+
**What changes.** A rename that meets both spellings now splits on whether the
21+
two values actually disagree:
22+
23+
- **Same value** (structural equality, so two separately-authored
24+
`{ status: 'stale' }` filters count as one declaration) → the alias carries
25+
nothing the canonical key does not, so it is **deleted** and the conversion
26+
emits its usual notice. Lossless hygiene, and it makes the transform
27+
idempotent in both shape and notices.
28+
- **Different values** → **both keys are kept** and no notice is emitted. Two
29+
spellings holding two different values is genuine author ambiguity, and an
30+
upgrade tool that silently picked the canonical one would be editing a
31+
configuration the customer never agreed to. The surviving pair is what lets
32+
the strict node-config gates refuse with a prescription that **names both
33+
keys** and asks for a decision.
34+
35+
`notify`'s nested `source: { object, id }` lift follows the same rule: a part
36+
that repeats its flat counterpart is redundant and `source` is dropped, while a
37+
part that disagrees leaves the node **entirely** untouched so `source` reaches
38+
the strict contract intact. (The `wait` node's loose-key lift is deliberately
39+
NOT covered — it moves keys between two locations rather than resolving two
40+
spellings of one slot.)
41+
42+
**What an author sees.** Metadata that named one slot twice with the same value
43+
loses the retired spelling at load and gains one deprecation notice per removal
44+
— the same notice a plain rename already emitted, so `objectstack validate`
45+
output is unchanged in kind. Metadata that named one slot twice with *different*
46+
values is unchanged by the conversion and is now refused by the strict
47+
node-config contracts with a message naming both keys; the fix is to decide
48+
which value is right, put it on the canonical key, and delete the alias. The
49+
batch-9 prescriptions were reworded accordingly: they no longer say the
50+
surviving twin is "dead" (true only under the old rule), because a key that now
51+
reaches that parse holds a value the canonical key does not.

‎packages/spec/src/automation/builtin-node-config.test.ts‎

Lines changed: 13 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -244,12 +244,18 @@ describe('MapConfigSchema — strict as of #4001 批 9', () => {
244244
expect(message).toMatch(/delete/i);
245245
});
246246

247-
it('rejects the SHADOWED alias the ADR-0087 conversion deliberately leaves behind', () => {
248-
// `renameConfigKey` does nothing when the canonical key is already
249-
// present, so `{ flowName, flow }` survives the load-path conversion
250-
// intact — and under `.strip` the dead twin was then deleted in silence at
251-
// this parse. That silence is the whole point of #4001: the author wrote
252-
// two names for one thing and the platform picked one without saying so.
253-
expect(MapConfigSchema.safeParse({ collection: '{r}', flowName: 'per_row', flow: 'ignored' }).success).toBe(false);
247+
it('rejects the AMBIGUOUS pair the ADR-0087 conversion deliberately leaves behind', () => {
248+
// Since #4923 `renameConfigKey` resolves `{ flowName, flow }` itself when
249+
// the two agree, so the pair that survives the load-path conversion to
250+
// reach this parse is the one naming two DIFFERENT flows. Under `.strip`
251+
// the loser was then deleted in silence — the whole point of #4001: the
252+
// author wrote two names for one thing and the platform picked one without
253+
// saying so. This is the case where picking would be a guess, so the
254+
// refusal names both keys instead.
255+
expect(MapConfigSchema.safeParse({ collection: '{r}', flowName: 'per_row', flow: 'per_row_v2' }).success)
256+
.toBe(false);
257+
const message = unknownKeyMessage(MapConfigSchema, { collection: '{r}', flowName: 'per_row', flow: 'per_row_v2' })!;
258+
expect(message).toContain('`flow`');
259+
expect(message).toContain('`flowName`');
254260
});
255261
});

‎packages/spec/src/automation/builtin-node-config.zod.ts‎

Lines changed: 28 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -84,11 +84,23 @@ const BUILTIN_NODE_CONFIG_HISTORY =
8484
* The two ADR-0087 D2 aliases every CRUD node shares.
8585
*
8686
* Both are retired SPELLINGS rather than typos, and both are rewritten at load
87-
* (`flow-node-crud-object-alias`, `flow-node-crud-filter-alias`), so a config
88-
* still carrying one at parse time carries the canonical key too —
89-
* `renameConfigKey` leaves a shadowed alias in place instead of clobbering the
90-
* winner. Hence each prescription answers both readings: the rename, and
91-
* "delete the dead twin".
87+
* (`flow-node-crud-object-alias`, `flow-node-crud-filter-alias`). So each
88+
* prescription has to answer two different readers, and since #4923 the split
89+
* between them is exact:
90+
*
91+
* - whoever **parses this contract directly** (never went through the load
92+
* path) wrote the retired spelling on its own → the fix is the rename;
93+
* - whoever **came through the load path** still has the alias only because
94+
* the conversion refused to resolve it, and it refuses in exactly one
95+
* situation: the canonical key is ALSO present and carries a DIFFERENT
96+
* value. An identical twin is deleted by the conversion now, so it can no
97+
* longer reach this parse.
98+
*
99+
* That second reader is why the prescription names BOTH keys and asks for a
100+
* decision rather than a deletion. Telling them "the canonical one already won,
101+
* delete yours" — true while a shadowed alias was left in place — would now be
102+
* advice to discard the one value the platform deliberately declined to discard
103+
* on their behalf.
92104
*
93105
* `object` also earns its entry on distance alone: `object` → `objectName` is
94106
* four edits against a threshold of two, so the suggester would say nothing at
@@ -97,13 +109,17 @@ const BUILTIN_NODE_CONFIG_HISTORY =
97109
const CRUD_ALIAS_GUIDANCE = {
98110
object:
99111
'The object slot is `objectName`. `object` was the last tenant of the `readAliasedConfig` executor shim; it '
100-
+ 'graduated into the ADR-0087 D2 conversion `flow-node-crud-object-alias` (#3796), which rewrites it at load — '
101-
+ 'so a surviving `object` means `objectName` already won and this key is dead. Delete it.',
112+
+ 'graduated into the ADR-0087 D2 conversion `flow-node-crud-object-alias` (#3796), which rewrites it at load. '
113+
+ 'If `objectName` is also present, this node names two different objects and the conversion left both keys '
114+
+ 'alone rather than picking for you (#4923) — decide which object this node acts on, put it on `objectName`, '
115+
+ 'and delete `object`.',
102116
filters:
103117
'The match map is `filter` (singular). `filters` was a consumer-side executor fallback that graduated into the '
104118
+ 'ADR-0087 D2 conversion `flow-node-crud-filter-alias`, which rewrites it at load; delete it once `filter` '
105-
+ 'carries the pairs. Beware the half-migrated shape: an empty `filter` next to a populated `filters` is what '
106-
+ 'made this alias dangerous enough to declare (#3810 — a match-everything write).',
119+
+ 'carries the pairs. If `filter` is also present, the two carry DIFFERENT match maps and the conversion kept '
120+
+ 'both rather than choosing (#4923) — reconcile them onto `filter`. Beware the half-migrated shape: an empty '
121+
+ '`filter` next to a populated `filters` is what made this alias dangerous enough to declare (#3810 — a '
122+
+ 'match-everything write).',
107123
} as const;
108124

109125
/**
@@ -457,7 +473,9 @@ export const MapConfigSchema = lazySchema(() => strictObject({
457473
flow:
458474
'The per-item subflow is named by `flowName`. `flow` was an undeclared executor fallback no schema or form '
459475
+ 'described; it graduated into the ADR-0087 D2 conversion `flow-node-map-flow-alias` (#4045), which rewrites '
460-
+ 'it at load — so a surviving `flow` means `flowName` already won and this key is dead. Delete it.',
476+
+ 'it at load. If `flowName` is also present, the two name DIFFERENT subflows and the conversion kept both '
477+
+ 'rather than picking one to run per item (#4923) — decide which flow this is, put it on `flowName`, and '
478+
+ 'delete `flow`.',
461479
},
462480
}, {
463481
/** The collection — a `{token}` template / bare variable name, or an inline array. */

‎packages/spec/src/automation/io-node-config.test.ts‎

Lines changed: 9 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -64,20 +64,23 @@ describe('NotifyConfigSchema — strict as of #4001 批 9', () => {
6464
['url', '/task/1', '`actionUrl`'],
6565
['source', { object: 'showcase_task', id: '1' }, '`sourceObject` + `sourceId`'],
6666
] as ReadonlyArray<[string, unknown, string]>)(
67-
'names the canonical key AND the dead-twin case for the retired `%s` alias',
67+
'names the canonical key AND the disagreeing-pair case for the retired `%s` alias',
6868
(key, value, canonical) => {
6969
const message = unknownKeyMessage(NotifyConfigSchema, {
7070
recipients: ['u1'], title: 'hi', [key]: value,
7171
});
7272
expect(message).toContain(canonical);
7373
// Both readings must be served: the ADR-0087 conversion rewrites this
7474
// key at load, so a config that still carries it at PARSE time also
75-
// carries the canonical key — `renameConfigKey` leaves a shadowed alias
76-
// in place rather than clobbering the winner. Without this half the
77-
// prescription ("rename it") is wrong for the population that actually
78-
// reaches this error.
75+
// carries the canonical key — and since #4923 it carries one holding a
76+
// DIFFERENT value, because an identical twin is deleted by the
77+
// conversion. Without this half the prescription ("rename it") is wrong
78+
// for the population that actually reaches this error.
7979
expect(message).toContain('flow-node-notify-config-aliases');
80-
expect(message).toMatch(/delete/i);
80+
expect(message).toMatch(/delete|reconcile/i);
81+
// The reconciliation reading has to name the OTHER key too, or the
82+
// author cannot see which two spellings disagree.
83+
expect(message).toMatch(/DIFFERENT|differ/i);
8184
},
8285
);
8386

‎packages/spec/src/automation/io-node-config.zod.ts‎

Lines changed: 25 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -74,34 +74,45 @@ const IO_NODE_CONFIG_HISTORY =
7474
*
7575
* Each is a RETIRED SPELLING, not a typo, so a bare "did you mean" would
7676
* under-serve it: `flow-node-notify-config-aliases` rewrites all five at load
77-
* (including the `registerFlow` rehydration seam), which means a config that
78-
* still carries one when it reaches this parse carries the canonical key too —
79-
* `renameConfigKey` leaves a SHADOWED alias in place rather than clobbering the
80-
* winner. So each prescription answers both readings: the rename, for whoever
81-
* parses this contract directly, and "delete the dead twin", for whoever came
82-
* through the load path.
77+
* (including the `registerFlow` rehydration seam). Since #4923 that rewrite
78+
* also DELETES a retired spelling that merely repeats the canonical key's
79+
* value, which sharpens what a surviving one means: it survived because the
80+
* canonical key is also present and says something DIFFERENT, and the
81+
* conversion will not pick between two values the author wrote.
82+
*
83+
* So each prescription answers both readings — the rename, for whoever parses
84+
* this contract directly, and a reconciliation naming BOTH keys, for whoever
85+
* came through the load path.
8386
*/
8487
const NOTIFY_KEY_GUIDANCE: Readonly<Record<string, string>> = {
8588
to:
8689
'The recipient slot is `recipients`. `to` is the pre-17 spelling, rewritten at load by the ADR-0087 D2 '
87-
+ 'conversion `flow-node-notify-config-aliases` — so if `recipients` is already present, the conversion left '
88-
+ '`to` behind as a dead twin (a shadowed alias is not clobbered) and it should be deleted.',
90+
+ 'conversion `flow-node-notify-config-aliases` — so if `recipients` is also present, the two name DIFFERENT '
91+
+ 'recipients and the conversion kept both rather than choosing who gets notified (#4923). Decide the '
92+
+ 'recipients, put them on `recipients`, and delete `to`.',
8993
subject:
9094
'The heading slot is `title`. `subject` is the pre-17 spelling rewritten at load by '
91-
+ '`flow-node-notify-config-aliases`; delete it once `title` carries the text.',
95+
+ '`flow-node-notify-config-aliases`; delete it once `title` carries the text. If `title` is also present with '
96+
+ 'DIFFERENT text, the conversion kept both rather than choosing (#4923) — reconcile them onto `title`.',
9297
body:
9398
'The body slot is `message`. `body` is the pre-17 spelling rewritten at load by '
94-
+ '`flow-node-notify-config-aliases`; delete it once `message` carries the text. (`body` IS canonical on an '
95-
+ '`http` node — the key is wrong only here.)',
99+
+ '`flow-node-notify-config-aliases`; delete it once `message` carries the text. If `message` is also present '
100+
+ 'with DIFFERENT text, the conversion kept both rather than choosing (#4923) — reconcile them onto `message`. '
101+
+ '(`body` IS canonical on an `http` node — the key is wrong only here.)',
96102
url:
97103
'The click-through slot is `actionUrl`. It was renamed at 17 because `url` elsewhere on the platform means '
98104
+ '"HTTP endpoint to call" (`http` node, webhooks), a different concept from an in-app click target. '
99-
+ '`flow-node-notify-config-aliases` rewrites it at load; delete it once `actionUrl` carries the link.',
105+
+ '`flow-node-notify-config-aliases` rewrites it at load; delete it once `actionUrl` carries the link. If '
106+
+ '`actionUrl` is also present with a DIFFERENT link, the conversion kept both rather than choosing where the '
107+
+ 'notification points (#4923) — reconcile them onto `actionUrl`.',
100108
source:
101109
'The click-through target is the flat PAIR `sourceObject` + `sourceId`, never a nested `source: { object, id }`. '
102110
+ '`flow-node-notify-config-aliases` lifts the nested shape at load and drops it once every part is accounted '
103-
+ 'for, so a surviving `source` means both flat keys were already set — delete it. Note the pair only takes '
104-
+ 'effect together: a half-specified target is dropped so the inbox never renders a dead link.',
111+
+ 'for, so a surviving `source` means a part of it holds a DIFFERENT value from the flat `sourceObject` / '
112+
+ '`sourceId` already in that slot, and the conversion declined to pick (#4923) — reconcile onto the flat pair '
113+
+ 'and delete `source`. '
114+
+ 'Note the pair only takes effect together: a half-specified target is dropped so the inbox never renders a '
115+
+ 'dead link.',
105116
};
106117

107118
// ─── notify ──────────────────────────────────────────────────────────

‎packages/spec/src/automation/schemaless-node-config.zod.ts‎

Lines changed: 14 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -119,10 +119,11 @@ const SCHEMALESS_NODE_CONFIG_HISTORY =
119119
* `functionName` and `input` are retired SPELLINGS that
120120
* `flow-node-script-config-aliases` rewrites at load, so — like the notify
121121
* family — a config still carrying one at parse time carries the canonical key
122-
* too (`renameConfigKey` leaves a shadowed alias alone). `input` earns its
123-
* entry twice over: edit distance would suggest `inputs` without ever saying
124-
* that `input` is *canonical* on `connector_action`'s `connectorConfig`, which
125-
* is where the spelling leaked in from and where it must NOT be changed.
122+
* too, and since #4923 it carries a canonical key holding a DIFFERENT value
123+
* (an identical twin is deleted by the conversion). `input` earns its entry
124+
* twice over: edit distance would suggest `inputs` without ever saying that
125+
* `input` is *canonical* on `connector_action`'s `connectorConfig`, which is
126+
* where the spelling leaked in from and where it must NOT be changed.
126127
*
127128
* The five `actionType`-branch keys need no entry here: `retiredKey()` puts the
128129
* prescription in the shape itself, which is strictly stronger (it also types
@@ -132,20 +133,25 @@ const SCHEMALESS_NODE_CONFIG_HISTORY =
132133
const SCRIPT_KEY_GUIDANCE: Readonly<Record<string, string>> = {
133134
functionName:
134135
'The callable reference is `function` (#1870). `functionName` was the AI/template-emitted alias, rewritten at '
135-
+ 'load by the ADR-0087 D2 conversion `flow-node-script-config-aliases`; if `function` is already present the '
136-
+ 'conversion left `functionName` behind as a dead twin — delete it.',
136+
+ 'load by the ADR-0087 D2 conversion `flow-node-script-config-aliases`. If `function` is also present, the two '
137+
+ 'name DIFFERENT callables and the conversion kept both rather than picking which one runs (#4923) — decide '
138+
+ 'which it is, put it on `function`, and delete `functionName`.',
137139
input:
138140
'The input map on a `script` node is `inputs` (plural). The singular `input` leaked in from '
139141
+ "`connector_action`, where `connectorConfig.input` is a DIFFERENT and canonical surface — do not \"fix\" that "
140-
+ 'one. `flow-node-script-config-aliases` rewrites this key at load; delete it once `inputs` carries the values.',
142+
+ 'one. `flow-node-script-config-aliases` rewrites this key at load; delete it once `inputs` carries the values. '
143+
+ 'If `inputs` is also present with DIFFERENT values, the conversion kept both rather than choosing (#4923) — '
144+
+ 'reconcile them onto `inputs`.',
141145
};
142146

143147
/** `subflow` prescriptions — one retired spelling, one wrong layer. */
144148
const SUBFLOW_KEY_GUIDANCE: Readonly<Record<string, string>> = {
145149
flow:
146150
'The invoked flow is named by `flowName`. `flow` was an undeclared executor fallback that no schema or form '
147151
+ 'ever described; it graduated into the ADR-0087 D2 conversion `flow-node-subflow-flow-alias` (#4278), which '
148-
+ 'rewrites it at load — so a surviving `flow` means `flowName` already won and this key is dead. Delete it.',
152+
+ 'rewrites it at load. If `flowName` is also present, the two name DIFFERENT flows and the conversion kept '
153+
+ 'both rather than picking which one this step invokes (#4923) — decide which it is, put it on `flowName`, '
154+
+ 'and delete `flow`.',
149155
timeoutMs:
150156
"A subflow step's timeout is the engine's per-node guard, so it belongs on the NODE, not in its config: "
151157
+ '`{ id, type: "subflow", timeoutMs: 30000, config: { … } }`. `FlowNodeSchema.timeoutMs` is the declared key.',

0 commit comments

Comments
 (0)