From fe9118f593e3b7605a963499605cb6dc2ffb9a38 Mon Sep 17 00:00:00 2001 From: Jack Zhuang <277994282+os-zhuang@users.noreply.github.com> Date: Sun, 28 Jun 2026 18:44:47 +0800 Subject: [PATCH] refactor(service-automation): deprecate non-canonical CRUD node config-key aliases MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Flow node `config` is `z.record(z.string(), z.unknown())` in spec, so the executor is the only runtime gate on key names. The CRUD nodes quietly accepted `object` for `objectName` and `filters` for `filter` via `cfg.canonical ?? cfg.alias` — a Postel's-law tolerance that fossilizes the wrong shape into a de-facto second contract and hides metadata-generation bugs. Converge on the canonical key (matching the `fields`/`fieldValues` decision: reject the wrong shape at author/publish time, don't absorb it at runtime). This is the deprecation-window step, not a removal: - add `readAliasedConfig()` — returns the canonical value, else the alias with a one-time `logger.warn` naming the canonical key (graph-lint can't reach flows already stored in prod that are never re-published; the warn covers that gap). - wire it into get/create/update/delete_record for `objectName`/`filter`. - fix the one authoring source that taught the alias: skills/objectstack-automation SKILL.md update_record example `object:` -> `objectName:`. No alias is straight-removed — all are plausibly present in stored prod flows. Removal is a tracked follow-up gated on the warn window + a cloud graph-lint rule. Siblings (notify to/subject/body, script inputs/function, wait duration/signal, assignment name/key, http-dispatcher trigger `object`) get the same treatment in follow-ups. Tests: service-automation 225 pass (incl. new crud-config-aliases.test.ts), spec 6600 pass, tsup DTS build clean. Co-Authored-By: Claude Opus 4.8 --- .../src/builtin/config-aliases.ts | 74 ++++++++++++ .../src/builtin/crud-config-aliases.test.ts | 109 ++++++++++++++++++ .../src/builtin/crud-nodes.ts | 17 +-- skills/objectstack-automation/SKILL.md | 2 +- 4 files changed, 194 insertions(+), 8 deletions(-) create mode 100644 packages/services/service-automation/src/builtin/config-aliases.ts create mode 100644 packages/services/service-automation/src/builtin/crud-config-aliases.test.ts diff --git a/packages/services/service-automation/src/builtin/config-aliases.ts b/packages/services/service-automation/src/builtin/config-aliases.ts new file mode 100644 index 0000000000..a4fe56174c --- /dev/null +++ b/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(); + +/** 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, + 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; +} diff --git a/packages/services/service-automation/src/builtin/crud-config-aliases.test.ts b/packages/services/service-automation/src/builtin/crud-config-aliases.test.ts new file mode 100644 index 0000000000..260dd92139 --- /dev/null +++ b/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) { + 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); + }); +}); diff --git a/packages/services/service-automation/src/builtin/crud-nodes.ts b/packages/services/service-automation/src/builtin/crud-nodes.ts index c2e7dd744a..89fd8474ad 100644 --- a/packages/services/service-automation/src/builtin/crud-nodes.ts +++ b/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; - 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; + const filter = interpolate(readAliasedConfig(cfg, 'get_record', 'filter', ['filters'], ctx.logger) ?? {}, variables, context) as Record; 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; - 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; @@ -134,10 +135,12 @@ export function registerCrudNodes(engine: AutomationEngine, ctx: PluginContext): }), async execute(node, variables, context) { const cfg = (node.config ?? {}) as Record; - 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; + const filter = interpolate(readAliasedConfig(cfg, 'update_record', 'filter', ['filters'], ctx.logger) ?? {}, variables, context) as Record; + // `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; 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; - 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; + const filter = interpolate(readAliasedConfig(cfg, 'delete_record', 'filter', ['filters'], ctx.logger) ?? {}, variables, context) as Record; const data = getData(); if (!data) return { success: true }; diff --git a/skills/objectstack-automation/SKILL.md b/skills/objectstack-automation/SKILL.md index 6e127c602e..d6111ace4f 100644 --- a/skills/objectstack-automation/SKILL.md +++ b/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' }, ],