fix(spec): ADR-0087 别名转换按值处置被遮蔽的旧拼法(等值删除、异值双留) (#4923)

.changeset/conversion-shadowed-alias-by-value.md

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

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

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

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

@@ -84,11 +84,23 @@ const BUILTIN_NODE_CONFIG_HISTORY =
  * The two ADR-0087 D2 aliases every CRUD node shares.
  *
  * Both are retired SPELLINGS rather than typos, and both are rewritten at load
- * (`flow-node-crud-object-alias`, `flow-node-crud-filter-alias`), so a config
- * still carrying one at parse time carries the canonical key too —
- * `renameConfigKey` leaves a shadowed alias in place instead of clobbering the
- * winner. Hence each prescription answers both readings: the rename, and
- * "delete the dead twin".
+ * (`flow-node-crud-object-alias`, `flow-node-crud-filter-alias`). So each
+ * prescription has to answer two different readers, and since #4923 the split
+ * between them is exact:
+ *
+ *  - whoever **parses this contract directly** (never went through the load
+ *    path) wrote the retired spelling on its own → the fix is the rename;
+ *  - whoever **came through the load path** still has the alias only because
+ *    the conversion refused to resolve it, and it refuses in exactly one
+ *    situation: the canonical key is ALSO present and carries a DIFFERENT
+ *    value. An identical twin is deleted by the conversion now, so it can no
+ *    longer reach this parse.
+ *
+ * That second reader is why the prescription names BOTH keys and asks for a
+ * decision rather than a deletion. Telling them "the canonical one already won,
+ * delete yours" — true while a shadowed alias was left in place — would now be
+ * advice to discard the one value the platform deliberately declined to discard
+ * on their behalf.
  *
  * `object` also earns its entry on distance alone: `object` → `objectName` is
  * four edits against a threshold of two, so the suggester would say nothing at
@@ -97,13 +109,17 @@ const BUILTIN_NODE_CONFIG_HISTORY =
 const CRUD_ALIAS_GUIDANCE = {
   object:
     'The object slot is `objectName`. `object` was the last tenant of the `readAliasedConfig` executor shim; it '
-    + 'graduated into the ADR-0087 D2 conversion `flow-node-crud-object-alias` (#3796), which rewrites it at load — '
-    + 'so a surviving `object` means `objectName` already won and this key is dead. Delete it.',
+    + 'graduated into the ADR-0087 D2 conversion `flow-node-crud-object-alias` (#3796), which rewrites it at load. '
+    + 'If `objectName` is also present, this node names two different objects and the conversion left both keys '
+    + 'alone rather than picking for you (#4923) — decide which object this node acts on, put it on `objectName`, '
+    + 'and delete `object`.',
   filters:
     'The match map is `filter` (singular). `filters` was a consumer-side executor fallback that graduated into the '
     + 'ADR-0087 D2 conversion `flow-node-crud-filter-alias`, which rewrites it at load; delete it once `filter` '
-    + 'carries the pairs. Beware the half-migrated shape: an empty `filter` next to a populated `filters` is what '
-    + 'made this alias dangerous enough to declare (#3810 — a match-everything write).',
+    + 'carries the pairs. If `filter` is also present, the two carry DIFFERENT match maps and the conversion kept '
+    + 'both rather than choosing (#4923) — reconcile them onto `filter`. Beware the half-migrated shape: an empty '
+    + '`filter` next to a populated `filters` is what made this alias dangerous enough to declare (#3810 — a '
+    + 'match-everything write).',
 } as const;
 
 /**
@@ -457,7 +473,9 @@ export const MapConfigSchema = lazySchema(() => strictObject({
     flow:
       'The per-item subflow is named by `flowName`. `flow` was an undeclared executor fallback no schema or form '
       + 'described; it graduated into the ADR-0087 D2 conversion `flow-node-map-flow-alias` (#4045), which rewrites '
-      + 'it at load — so a surviving `flow` means `flowName` already won and this key is dead. Delete it.',
+      + 'it at load. If `flowName` is also present, the two name DIFFERENT subflows and the conversion kept both '
+      + 'rather than picking one to run per item (#4923) — decide which flow this is, put it on `flowName`, and '
+      + 'delete `flow`.',
   },
 }, {
   /** The collection — a `{token}` template / bare variable name, or an inline array. */

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

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

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

@@ -74,34 +74,45 @@ const IO_NODE_CONFIG_HISTORY =
  *
  * Each is a RETIRED SPELLING, not a typo, so a bare "did you mean" would
  * under-serve it: `flow-node-notify-config-aliases` rewrites all five at load
- * (including the `registerFlow` rehydration seam), which means a config that
- * still carries one when it reaches this parse carries the canonical key too —
- * `renameConfigKey` leaves a SHADOWED alias in place rather than clobbering the
- * winner. So each prescription answers both readings: the rename, for whoever
- * parses this contract directly, and "delete the dead twin", for whoever came
- * through the load path.
+ * (including the `registerFlow` rehydration seam). Since #4923 that rewrite
+ * also DELETES a retired spelling that merely repeats the canonical key's
+ * value, which sharpens what a surviving one means: it survived because the
+ * canonical key is also present and says something DIFFERENT, and the
+ * conversion will not pick between two values the author wrote.
+ *
+ * So each prescription answers both readings — the rename, for whoever parses
+ * this contract directly, and a reconciliation naming BOTH keys, for whoever
+ * came through the load path.
  */
 const NOTIFY_KEY_GUIDANCE: Readonly<Record<string, string>> = {
   to:
     'The recipient slot is `recipients`. `to` is the pre-17 spelling, rewritten at load by the ADR-0087 D2 '
-    + 'conversion `flow-node-notify-config-aliases` — so if `recipients` is already present, the conversion left '
-    + '`to` behind as a dead twin (a shadowed alias is not clobbered) and it should be deleted.',
+    + 'conversion `flow-node-notify-config-aliases` — so if `recipients` is also present, the two name DIFFERENT '
+    + 'recipients and the conversion kept both rather than choosing who gets notified (#4923). Decide the '
+    + 'recipients, put them on `recipients`, and delete `to`.',
   subject:
     'The heading slot is `title`. `subject` is the pre-17 spelling rewritten at load by '
-    + '`flow-node-notify-config-aliases`; delete it once `title` carries the text.',
+    + '`flow-node-notify-config-aliases`; delete it once `title` carries the text. If `title` is also present with '
+    + 'DIFFERENT text, the conversion kept both rather than choosing (#4923) — reconcile them onto `title`.',
   body:
     'The body slot is `message`. `body` is the pre-17 spelling rewritten at load by '
-    + '`flow-node-notify-config-aliases`; delete it once `message` carries the text. (`body` IS canonical on an '
-    + '`http` node — the key is wrong only here.)',
+    + '`flow-node-notify-config-aliases`; delete it once `message` carries the text. If `message` is also present '
+    + 'with DIFFERENT text, the conversion kept both rather than choosing (#4923) — reconcile them onto `message`. '
+    + '(`body` IS canonical on an `http` node — the key is wrong only here.)',
   url:
     'The click-through slot is `actionUrl`. It was renamed at 17 because `url` elsewhere on the platform means '
     + '"HTTP endpoint to call" (`http` node, webhooks), a different concept from an in-app click target. '
-    + '`flow-node-notify-config-aliases` rewrites it at load; delete it once `actionUrl` carries the link.',
+    + '`flow-node-notify-config-aliases` rewrites it at load; delete it once `actionUrl` carries the link. If '
+    + '`actionUrl` is also present with a DIFFERENT link, the conversion kept both rather than choosing where the '
+    + 'notification points (#4923) — reconcile them onto `actionUrl`.',
   source:
     'The click-through target is the flat PAIR `sourceObject` + `sourceId`, never a nested `source: { object, id }`. '
     + '`flow-node-notify-config-aliases` lifts the nested shape at load and drops it once every part is accounted '
-    + 'for, so a surviving `source` means both flat keys were already set — delete it. Note the pair only takes '
-    + 'effect together: a half-specified target is dropped so the inbox never renders a dead link.',
+    + 'for, so a surviving `source` means a part of it holds a DIFFERENT value from the flat `sourceObject` / '
+    + '`sourceId` already in that slot, and the conversion declined to pick (#4923) — reconcile onto the flat pair '
+    + 'and delete `source`. '
+    + 'Note the pair only takes effect together: a half-specified target is dropped so the inbox never renders a '
+    + 'dead link.',
 };
 
 // ─── notify ──────────────────────────────────────────────────────────

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

@@ -119,10 +119,11 @@ const SCHEMALESS_NODE_CONFIG_HISTORY =
  * `functionName` and `input` are retired SPELLINGS that
  * `flow-node-script-config-aliases` rewrites at load, so — like the notify
  * family — a config still carrying one at parse time carries the canonical key
- * too (`renameConfigKey` leaves a shadowed alias alone). `input` earns its
- * entry twice over: edit distance would suggest `inputs` without ever saying
- * that `input` is *canonical* on `connector_action`'s `connectorConfig`, which
- * is where the spelling leaked in from and where it must NOT be changed.
+ * too, and since #4923 it carries a canonical key holding a DIFFERENT value
+ * (an identical twin is deleted by the conversion). `input` earns its entry
+ * twice over: edit distance would suggest `inputs` without ever saying that
+ * `input` is *canonical* on `connector_action`'s `connectorConfig`, which is
+ * where the spelling leaked in from and where it must NOT be changed.
  *
  * The five `actionType`-branch keys need no entry here: `retiredKey()` puts the
  * prescription in the shape itself, which is strictly stronger (it also types
@@ -132,20 +133,25 @@ const SCHEMALESS_NODE_CONFIG_HISTORY =
 const SCRIPT_KEY_GUIDANCE: Readonly<Record<string, string>> = {
   functionName:
     'The callable reference is `function` (#1870). `functionName` was the AI/template-emitted alias, rewritten at '
-    + 'load by the ADR-0087 D2 conversion `flow-node-script-config-aliases`; if `function` is already present the '
-    + 'conversion left `functionName` behind as a dead twin — delete it.',
+    + 'load by the ADR-0087 D2 conversion `flow-node-script-config-aliases`. If `function` is also present, the two '
+    + 'name DIFFERENT callables and the conversion kept both rather than picking which one runs (#4923) — decide '
+    + 'which it is, put it on `function`, and delete `functionName`.',
   input:
     'The input map on a `script` node is `inputs` (plural). The singular `input` leaked in from '
     + "`connector_action`, where `connectorConfig.input` is a DIFFERENT and canonical surface — do not \"fix\" that "
-    + 'one. `flow-node-script-config-aliases` rewrites this key at load; delete it once `inputs` carries the values.',
+    + 'one. `flow-node-script-config-aliases` rewrites this key at load; delete it once `inputs` carries the values. '
+    + 'If `inputs` is also present with DIFFERENT values, the conversion kept both rather than choosing (#4923) — '
+    + 'reconcile them onto `inputs`.',
 };
 
 /** `subflow` prescriptions — one retired spelling, one wrong layer. */
 const SUBFLOW_KEY_GUIDANCE: Readonly<Record<string, string>> = {
   flow:
     'The invoked flow is named by `flowName`. `flow` was an undeclared executor fallback that no schema or form '
     + 'ever described; it graduated into the ADR-0087 D2 conversion `flow-node-subflow-flow-alias` (#4278), which '
-    + 'rewrites it at load — so a surviving `flow` means `flowName` already won and this key is dead. Delete it.',
+    + 'rewrites it at load. If `flowName` is also present, the two name DIFFERENT flows and the conversion kept '
+    + 'both rather than picking which one this step invokes (#4923) — decide which it is, put it on `flowName`, '
+    + 'and delete `flow`.',
   timeoutMs:
     "A subflow step's timeout is the engine's per-node guard, so it belongs on the NODE, not in its config: "
     + '`{ id, type: "subflow", timeoutMs: 30000, config: { … } }`. `FlowNodeSchema.timeoutMs` is the declared key.',