refactor(service-automation): deprecate non-canonical CRUD node config-key aliases (#2425)

Author: os-zhuang

packages/services/service-automation/src/builtin/config-aliases.ts

@@ -0,0 +1,74 @@
+// Copyright (c) 2025 ObjectStack. Licensed under the Apache-2.0 license.
+
+/**
+ * Deprecation shim for non-canonical flow-node `config` keys.
+ *
+ * `FlowNodeSchema.config` is an unconstrained `z.record(z.string(), z.unknown())`
+ * (packages/spec/src/automation/flow.zod.ts), so the spec blesses *no* particular
+ * key — the executor is the only thing that gives `config` a shape at runtime.
+ * Historically several built-in executors quietly accepted a non-canonical alias
+ * via `cfg.canonical ?? cfg.alias` (e.g. `object` for `objectName`, `filters` for
+ * `filter`). That Postel's-law tolerance fossilizes the wrong shape into a
+ * de-facto second contract and hides metadata-generation bugs: a flow authored
+ * with the wrong key "just works" at runtime, so nothing ever flags it.
+ *
+ * The convergence (mirroring the `fields` / `fieldValues` decision) is to make
+ * the canonical key the *single* contract and reject the wrong shape at
+ * author/publish time:
+ *   • the cloud `graph-lint` gate rejects the alias when a flow is published, and
+ *   • the authoring sources (skills / example flows) only ever emit the canonical.
+ *
+ * graph-lint only runs at publish, so it cannot reach flows already stored in
+ * prod that are never re-published. {@link readAliasedConfig} closes that gap for
+ * the **deprecation window**: it keeps accepting the alias (so live flows keep
+ * running) but emits a one-time `logger.warn` per alias steering the author to
+ * the canonical key. Removal of the alias paths is tracked as a follow-up and
+ * happens once the window has elapsed and graph-lint has been enforcing.
+ *
+ * @see crud-nodes.ts for the first call sites (`objectName`, `filter`).
+ */
+
+/** One-time-warning ledger, keyed by `${nodeType}:${canonical}<-${alias}`. */
+const warnedAliases = new Set<string>();
+
+/** Test-only: clear the one-time-warning ledger so each test starts fresh. */
+export function __resetAliasDeprecationWarnings(): void {
+  warnedAliases.clear();
+}
+
+/**
+ * Read a node-config value by its **canonical** key, tolerating deprecated
+ * aliases for one deprecation window.
+ *
+ * Returns the value under `canonical` when present; otherwise the first present
+ * `alias` (warning once), otherwise `undefined`. "Present" means `!= null`, so
+ * the fall-through matches the `cfg.canonical ?? cfg.alias` semantics it replaces
+ * — callers keep applying their own default (`?? {}` / `?? ''`).
+ *
+ * @deprecated The alias paths exist only to keep already-stored flows running.
+ *   Author flows with the canonical key; the aliases will be removed.
+ */
+export function readAliasedConfig(
+  cfg: Record<string, unknown>,
+  nodeType: string,
+  canonical: string,
+  aliases: readonly string[],
+  logger: { warn(message: string): void },
+): unknown {
+  if (cfg[canonical] != null) return cfg[canonical];
+  for (const alias of aliases) {
+    if (cfg[alias] != null) {
+      const key = `${nodeType}:${canonical}<-${alias}`;
+      if (!warnedAliases.has(key)) {
+        warnedAliases.add(key);
+        logger.warn(
+          `[${nodeType}] config key '${alias}' is a deprecated alias of '${canonical}'. ` +
+            `Rename it to '${canonical}' — the alias still works but is deprecated, rejected at ` +
+            `publish time by graph-lint, and will be removed in a future release.`,
+        );
+      }
+      return cfg[alias];
+    }
+  }
+  return undefined;
+}

packages/services/service-automation/src/builtin/crud-config-aliases.test.ts

@@ -0,0 +1,109 @@
+// Copyright (c) 2025 ObjectStack. Licensed under the Apache-2.0 license.
+
+/**
+ * Deprecation window for non-canonical `config` keys on the CRUD nodes
+ * (`object` → `objectName`, `filters` → `filter`). The alias keeps working so
+ * already-stored flows keep running, but emits a one-time `logger.warn` steering
+ * the author to the canonical key. See config-aliases.ts.
+ */
+import { describe, it, expect, beforeEach } from 'vitest';
+import { AutomationEngine } from '../engine.js';
+import { registerCrudNodes } from './crud-nodes.js';
+import { __resetAliasDeprecationWarnings } from './config-aliases.js';
+
+function silentLogger(): any {
+  const l: any = { info() {}, warn() {}, error() {}, debug() {} };
+  l.child = () => l;
+  return l;
+}
+
+function collectingLogger(warns: string[]): any {
+  const l: any = { info() {}, warn(m: string) { warns.push(m); }, error() {}, debug() {} };
+  l.child = () => l;
+  return l;
+}
+
+function fakeData() {
+  const calls: Array<{ op: string; obj: string; opts?: any }> = [];
+  const data: any = {
+    async find(obj: string, opts: any) { calls.push({ op: 'find', obj, opts }); return [{ id: 'r1' }]; },
+    async findOne(obj: string, opts: any) { calls.push({ op: 'findOne', obj, opts }); return { id: 'r1' }; },
+    async insert(obj: string, fields: any) { calls.push({ op: 'insert', obj, opts: { fields } }); return { id: `${obj}_1`, ...fields }; },
+    async update(obj: string, fields: any, opts: any) { calls.push({ op: 'update', obj, opts: { ...opts, fields } }); return { ok: true }; },
+    async delete(obj: string, opts: any) { calls.push({ op: 'delete', obj, opts }); return { ok: true }; },
+  };
+  return { data, calls };
+}
+
+const ctxWith = (data: any, logger: any): any => ({
+  logger,
+  getService: (n: string) => (n === 'data' ? data : undefined),
+});
+
+function getRecordFlow(config: Record<string, unknown>) {
+  return {
+    name: 'gr', label: 'G', type: 'autolaunched',
+    nodes: [
+      { id: 'start', type: 'start', label: 'Start' },
+      { id: 'g', type: 'get_record', label: 'Get', config },
+      { id: 'end', type: 'end', label: 'End' },
+    ],
+    edges: [
+      { id: 'e1', source: 'start', target: 'g' },
+      { id: 'e2', source: 'g', target: 'end' },
+    ],
+  } as any;
+}
+
+describe('CRUD config-key alias deprecation (object→objectName, filters→filter)', () => {
+  beforeEach(() => __resetAliasDeprecationWarnings());
+
+  it('still resolves the deprecated `object` + `filters` aliases at runtime', async () => {
+    const engine = new AutomationEngine(silentLogger());
+    const { data, calls } = fakeData();
+    const warns: string[] = [];
+    registerCrudNodes(engine, ctxWith(data, collectingLogger(warns)));
+
+    engine.registerFlow('gr', getRecordFlow({ object: 'crm_lead', filters: { id: 'L1' }, outputVariable: 'lead' }));
+    const res = await engine.execute('gr');
+
+    expect(res.success).toBe(true);
+    // The alias values reached the data engine unchanged.
+    expect(calls).toHaveLength(1);
+    expect(calls[0].obj).toBe('crm_lead');
+    expect(calls[0].opts.where).toEqual({ id: 'L1' });
+  });
+
+  it('warns once per alias, naming the canonical key', async () => {
+    const engine = new AutomationEngine(silentLogger());
+    const { data } = fakeData();
+    const warns: string[] = [];
+    registerCrudNodes(engine, ctxWith(data, collectingLogger(warns)));
+
+    engine.registerFlow('gr', getRecordFlow({ object: 'crm_lead', filters: { id: 'L1' } }));
+    await engine.execute('gr');
+
+    const objectWarn = warns.find((w) => w.includes("'object'") && w.includes("'objectName'"));
+    const filterWarn = warns.find((w) => w.includes("'filters'") && w.includes("'filter'"));
+    expect(objectWarn).toBeTruthy();
+    expect(filterWarn).toBeTruthy();
+
+    // Second run in the same process must NOT warn again (one-time per alias).
+    const before = warns.length;
+    await engine.execute('gr');
+    expect(warns.length).toBe(before);
+  });
+
+  it('does NOT warn when the canonical keys are used', async () => {
+    const engine = new AutomationEngine(silentLogger());
+    const { data } = fakeData();
+    const warns: string[] = [];
+    registerCrudNodes(engine, ctxWith(data, collectingLogger(warns)));
+
+    engine.registerFlow('gr', getRecordFlow({ objectName: 'crm_lead', filter: { id: 'L1' }, outputVariable: 'lead' }));
+    const res = await engine.execute('gr');
+
+    expect(res.success).toBe(true);
+    expect(warns).toHaveLength(0);
+  });
+});

packages/services/service-automation/src/builtin/crud-nodes.ts

@@ -5,6 +5,7 @@ import { defineActionDescriptor } from '@objectstack/spec/automation';
 import type { IDataEngine } from '@objectstack/spec/contracts';
 import type { AutomationEngine } from '../engine.js';
 import { interpolate } from './template.js';
+import { readAliasedConfig } from './config-aliases.js';
 import { resolveRunDataContext } from '../runtime-identity.js';
 
 /**
@@ -43,10 +44,10 @@ export function registerCrudNodes(engine: AutomationEngine, ctx: PluginContext):
             }),
             async execute(node, variables, context) {
                 const cfg = (node.config ?? {}) as Record<string, unknown>;
-                const objectName = String(cfg.objectName ?? cfg.object ?? '');
+                const objectName = String(readAliasedConfig(cfg, 'get_record', 'objectName', ['object'], ctx.logger) ?? '');
                 if (!objectName) return { success: false, error: 'get_record: objectName required' };
 
-                const filter = interpolate(cfg.filter ?? cfg.filters ?? {}, variables, context) as Record<string, unknown>;
+                const filter = interpolate(readAliasedConfig(cfg, 'get_record', 'filter', ['filters'], ctx.logger) ?? {}, variables, context) as Record<string, unknown>;
                 const fields = cfg.fields as string[] | undefined;
                 const limit = typeof cfg.limit === 'number' ? cfg.limit : undefined;
                 const outputVariable = cfg.outputVariable as string | undefined;
@@ -85,7 +86,7 @@ export function registerCrudNodes(engine: AutomationEngine, ctx: PluginContext):
             }),
             async execute(node, variables, context) {
                 const cfg = (node.config ?? {}) as Record<string, unknown>;
-                const objectName = String(cfg.objectName ?? cfg.object ?? '');
+                const objectName = String(readAliasedConfig(cfg, 'create_record', 'objectName', ['object'], ctx.logger) ?? '');
                 if (!objectName) return { success: false, error: 'create_record: objectName required' };
 
                 const fields = interpolate(cfg.fields ?? {}, variables, context) as Record<string, unknown>;
@@ -134,10 +135,12 @@ export function registerCrudNodes(engine: AutomationEngine, ctx: PluginContext):
             }),
             async execute(node, variables, context) {
                 const cfg = (node.config ?? {}) as Record<string, unknown>;
-                const objectName = String(cfg.objectName ?? cfg.object ?? '');
+                const objectName = String(readAliasedConfig(cfg, 'update_record', 'objectName', ['object'], ctx.logger) ?? '');
                 if (!objectName) return { success: false, error: 'update_record: objectName required' };
 
-                const filter = interpolate(cfg.filter ?? cfg.filters ?? {}, variables, context) as Record<string, unknown>;
+                const filter = interpolate(readAliasedConfig(cfg, 'update_record', 'filter', ['filters'], ctx.logger) ?? {}, variables, context) as Record<string, unknown>;
+                // `fields` is the single canonical write-map key — no alias (the wrong key
+                // `fieldValues` is corrected at the authoring source + rejected by graph-lint).
                 const fields = interpolate(cfg.fields ?? {}, variables, context) as Record<string, unknown>;
 
                 const data = getData();
@@ -167,10 +170,10 @@ export function registerCrudNodes(engine: AutomationEngine, ctx: PluginContext):
             }),
             async execute(node, variables, context) {
                 const cfg = (node.config ?? {}) as Record<string, unknown>;
-                const objectName = String(cfg.objectName ?? cfg.object ?? '');
+                const objectName = String(readAliasedConfig(cfg, 'delete_record', 'objectName', ['object'], ctx.logger) ?? '');
                 if (!objectName) return { success: false, error: 'delete_record: objectName required' };
 
-                const filter = interpolate(cfg.filter ?? cfg.filters ?? {}, variables, context) as Record<string, unknown>;
+                const filter = interpolate(readAliasedConfig(cfg, 'delete_record', 'filter', ['filters'], ctx.logger) ?? {}, variables, context) as Record<string, unknown>;
 
                 const data = getData();
                 if (!data) return { success: true };

skills/objectstack-automation/SKILL.md

@@ -278,7 +278,7 @@ is one diagram a reviewer (or AI) can read end-to-end.
       },
     },
     { id: 'mark_won', type: 'update_record',
-      config: { object: 'opportunity', filter: { id: '{record.id}' }, fields: { stage: 'closed_won' } } },
+      config: { objectName: 'opportunity', filter: { id: '{record.id}' }, fields: { stage: 'closed_won' } } },
     { id: 'approved', type: 'end' },
     { id: 'rejected', type: 'end' },
   ],