diff --git a/.changeset/agents-pd12-shim-retired.md b/.changeset/agents-pd12-shim-retired.md new file mode 100644 index 0000000000..ef0cd0de5a --- /dev/null +++ b/.changeset/agents-pd12-shim-retired.md @@ -0,0 +1,4 @@ +--- +--- + +docs(agents): PD #12 records the #3796 endgame — all seven flow-node config aliases graduated into the protocol-17 conversion layer and the `readAliasedConfig` shim is deleted; new aliases go straight to conversion entries, never executor shims — releases nothing. diff --git a/.changeset/flow-node-config-alias-graduation.md b/.changeset/flow-node-config-alias-graduation.md new file mode 100644 index 0000000000..e7beb06599 --- /dev/null +++ b/.changeset/flow-node-config-alias-graduation.md @@ -0,0 +1,49 @@ +--- +"@objectstack/spec": minor +"@objectstack/service-automation": minor +--- + +feat(spec,automation): graduate the seven flow-node config key aliases into the conversion layer — the `readAliasedConfig` shim retires with them (#3796) + +`FlowNodeSchema.config` is an unconstrained record, so the executors were the +only statement of which config key is canonical — and seven deprecated aliases +lived there as tolerance the spec never declared: one behind the +`readAliasedConfig` deprecation shim (warned, ledgered), six as open-coded +`??` fallbacks (no warning, no ledger, no retirement path). All seven now +graduate into the ADR-0087 D2 conversion layer as protocol-17 **live-window** +entries: a stored flow authored with an alias is rewritten to the canonical +key at load — `defineStack` / `validate` / `lint` and the +`AutomationEngine.registerFlow` rehydration seam alike — with a structured +`ConversionNotice` per rewrite, and the executors read the canonical keys +only. The shim (`service-automation/src/builtin/config-aliases.ts`) is empty +and deleted. + +FROM → TO (per node type; conversion entry in parentheses): + +- `get_record`/`create_record`/`update_record`/`delete_record`: + `config.object` → `config.objectName` (`flow-node-crud-object-alias`) +- `notify`: `config.to` → `config.recipients`, `config.subject` → + `config.title`, `config.body` → `config.message`, `config.url` → + `config.actionUrl` (`flow-node-notify-config-aliases`) +- `script`: `config.functionName` → `config.function`, `config.input` → + `config.inputs` (`flow-node-script-config-aliases`) + +One-line fix: rename the key in your flow source — values are unchanged; `os +migrate meta --from 16` rewrites all seven mechanically. Until then nothing +breaks: the protocol-17 loader accepts and converts the old shape (window +retires in 18). + +`actionUrl` (not `url`) is the deliberate canonical of its pair, resolving a +contradiction where the notify descriptor documented `url` as canonical while +the executor, tests, and examples preferred `actionUrl`: the whole downstream +chain already uses that name (`sys_notification.action_url`, the +channel-dispatch contract, the REST notification read model), and `url` +elsewhere in the platform means "HTTP endpoint to call" (`http` node, +webhooks) — a different concept from this in-app click-through target. The +executor precedence already put `actionUrl` first, so the choice is +behaviour-preserving; the `notify` descriptor's `configSchema` now documents +`actionUrl`. + +Callers that hand a node config **directly** to an executor (bypassing +`registerFlow`) no longer get alias resolution — build the config with the +canonical keys. diff --git a/AGENTS.md b/AGENTS.md index a7522c118c..60b9ec7fae 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -73,7 +73,7 @@ Other scripts: `objectui:bump` (pull only), `objectui:build`, `objectui:clean`. When renaming a legacy var, use `readEnvWithDeprecation('OS_NEW', 'LEGACY')` from `@objectstack/types` (keeps legacy working one release). Third-party exceptions kept as-is: `NODE_ENV`, `HOME`, `OPENAI_API_KEY`, `TURSO_*`, OAuth `*_CLIENT_ID/SECRET`, `RESEND_API_KEY`, `POSTMARK_TOKEN`, `AI_GATEWAY_*`, `SMTP_*`. See #1382. 10. **File issues for out-of-scope findings — don't silently expand scope or leave them buried.** When you hit a bug, gap, or unenforced capability that's unrelated to the current task, or too large to fix in scope, open a GitHub issue (`gh issue create`) with a clear repro/decision and link it from your PR. Corollary: **never advertise or demo a capability the runtime doesn't actually deliver** (declared ≠ enforced) — fix it, trim it, or file an issue, but don't fake coverage. Example: the spec once declared 9 validation-rule types while the write-path validator enforced only 3 (`state_machine`/`script`/`cross_field`); the gap was filed as #1475 rather than demoed in the showcase, then closed by **trimming** what could never be enforced (`unique`/`async`/`custom`) and **implementing** the rest — the spec now declares 6 and `rule-validator.ts` handles all 6. Note how narrow that claim stayed even so: the evaluator was wired into insert and single-id update only, so a bulk `updateMany` silently skipped every rule — a second `declared ≠ enforced` gap one layer down, at the **call site** rather than the `switch`; filed as #3106 and closed by evaluating the bulk match set per row. A `case` label is not enforcement; check the **call site**. 11. **Worktree-first — never edit on the shared `main` checkout.** This repo is edited by **multiple agents at once**; the shared `main` tree has its HEAD switched and reset *under you*, silently clobbering uncommitted work. Before your **first file edit**, you MUST be in a dedicated worktree on a feature branch: `git worktree add ../objectstack- -b main && cd ../objectstack- && pnpm install`. A PreToolUse hook (`.claude/hooks/guard-main-checkout.sh`) **enforces** this — it blocks `Edit`/`Write`/`NotebookEdit` unless the edited file is in a dedicated **worktree** — a feature branch on the *shared* checkout is **not** enough (it still gets switched under you) — and it checks the **edited file's own repo**, so sibling repos (`objectui`/`cloud`) you touch are covered too (override for a deliberate non-task fix with `OS_ALLOW_MAIN_EDITS=1`). Full playbook below. -12. **Contract-first — fix the metadata, not the runtime.** This is a metadata-driven framework: `packages/spec` is the one contract between metadata *producers* and the runtime/renderers that *consume* it. When a piece of metadata "doesn't work," ask **first**: *is it spec-compliant? is this the long-term-correct direction?* If the metadata is wrong, fix it at the **producer** and **reject it at authoring/publish** (validation / lint) so the error surfaces loudly — do **not** add a lenient alias or `??` fallback in the consumer (a node executor, the REST layer, a renderer) to tolerate off-spec input. A tolerant fallback fossilizes the wrong convention into a second de-facto contract, dilutes the spec, and hides the producer's bug — one strict contract beats N dialects. This is an **internal** contract (we own both ends), so "be liberal in what you accept" (Postel) does **not** apply — that's for untrusted boundaries. Change the **spec** only when the spec itself is genuinely wrong, and then deliberately (edit the Zod schema + migrate), never by accreting consumer-side fallbacks. The `cfg.filter ?? cfg.filters` / `cfg.objectName ?? cfg.object` fallbacks the flow executors once carried are **debt to pay down, not a pattern to copy** — and the way they are being paid down is the pattern to copy. `filters` → `filter` has **graduated** into the ADR-0087 D2 conversion layer (`flow-node-crud-filter-alias`): rewritten to the canonical key at load, including the `AutomationEngine.registerFlow` rehydration seam, so the CRUD executors read `cfg.filter` directly and no consumer-side fallback survives. `object` → `objectName` is mid-retirement behind `readAliasedConfig` (`packages/services/service-automation/src/builtin/config-aliases.ts`) — canonical wins, the alias keeps already-stored flows running, and each one warns once so authors get steered off it — with graduation into the conversion layer as the next step. When you must tolerate an alias at all, put it behind that shim (never a bare `??`) so it is declared, warned, lintable by `graph-lint` and *removable on a schedule*; stragglers still open-coded are tracked in #3796. *Worked example:* an AI-authored `create_record` used `fieldValues` / `today()` / `{{trigger.record.id}}` while the executor reads `fields` / `{TODAY()}` / `{record.id}` → the fix was correcting the authoring skill + a publish-gate lint that rejects the wrong shape (cloud#688), **not** a `cfg.fields ?? cfg.fieldValues` runtime alias (framework#2419, rejected). Strengthens #5. +12. **Contract-first — fix the metadata, not the runtime.** This is a metadata-driven framework: `packages/spec` is the one contract between metadata *producers* and the runtime/renderers that *consume* it. When a piece of metadata "doesn't work," ask **first**: *is it spec-compliant? is this the long-term-correct direction?* If the metadata is wrong, fix it at the **producer** and **reject it at authoring/publish** (validation / lint) so the error surfaces loudly — do **not** add a lenient alias or `??` fallback in the consumer (a node executor, the REST layer, a renderer) to tolerate off-spec input. A tolerant fallback fossilizes the wrong convention into a second de-facto contract, dilutes the spec, and hides the producer's bug — one strict contract beats N dialects. This is an **internal** contract (we own both ends), so "be liberal in what you accept" (Postel) does **not** apply — that's for untrusted boundaries. Change the **spec** only when the spec itself is genuinely wrong, and then deliberately (edit the Zod schema + migrate), never by accreting consumer-side fallbacks. The `cfg.filter ?? cfg.filters` / `cfg.objectName ?? cfg.object` fallbacks the flow executors once carried are **debt to pay down, not a pattern to copy** — and the way they are being paid down is the pattern to copy. `filters` → `filter` has **graduated** into the ADR-0087 D2 conversion layer (`flow-node-crud-filter-alias`): rewritten to the canonical key at load, including the `AutomationEngine.registerFlow` rehydration seam, so the CRUD executors read `cfg.filter` directly and no consumer-side fallback survives. `object` → `objectName` and the six open-coded stragglers #3796 tracked (notify `to`/`subject`/`body`/`url`, script `functionName`/`input`) graduated the same way at protocol 17 (`flow-node-crud-object-alias`, `flow-node-notify-config-aliases`, `flow-node-script-config-aliases`), emptying the `readAliasedConfig` executor shim — deleted with them. When you must tolerate an alias at all, declare it as a conversion-layer entry (never a bare `??`, and no new executor shims) so it is declared, loud, tested, and *removable on a schedule*. *Worked example:* an AI-authored `create_record` used `fieldValues` / `today()` / `{{trigger.record.id}}` while the executor reads `fields` / `{TODAY()}` / `{record.id}` → the fix was correcting the authoring skill + a publish-gate lint that rejects the wrong shape (cloud#688), **not** a `cfg.fields ?? cfg.fieldValues` runtime alias (framework#2419, rejected). Strengthens #5. --- diff --git a/content/docs/getting-started/common-patterns.mdx b/content/docs/getting-started/common-patterns.mdx index bed2f6fc9a..626bae818a 100644 --- a/content/docs/getting-started/common-patterns.mdx +++ b/content/docs/getting-started/common-patterns.mdx @@ -276,10 +276,10 @@ export const assignmentNotification = defineFlow({ label: 'Notify Assignee', config: { // String fields use single-brace {…} templates, not {{…}}. - // `to` takes recipient USER IDs — a lookup value interpolates to the id. - to: '{record.assigned_to}', - subject: 'Task Assigned: {record.title}', - body: 'You have been assigned to task "{record.title}".' + // `recipients` takes recipient USER IDs — a lookup value interpolates to the id. + recipients: '{record.assigned_to}', + title: 'Task Assigned: {record.title}', + message: 'You have been assigned to task "{record.title}".' } }, { id: 'end', type: 'end', label: 'End' } @@ -354,9 +354,9 @@ Define a multi-step approval flow for records. type: 'notify', label: 'Request Approval', config: { - to: '{record.submitted_by}', - subject: 'Expense approval needed: {record.title}', - body: 'Expense "{record.title}" needs manual approval.' + recipients: '{record.submitted_by}', + title: 'Expense approval needed: {record.title}', + message: 'Expense "{record.title}" needs manual approval.' } }, { id: 'end', type: 'end', label: 'End' } diff --git a/content/docs/releases/v17.mdx b/content/docs/releases/v17.mdx index 87e690d803..6133fc3c44 100644 --- a/content/docs/releases/v17.mdx +++ b/content/docs/releases/v17.mdx @@ -219,6 +219,33 @@ existed to warn when an author declared both an alias and its canonical key, which the parse now rejects outright. Delete the import; there is no replacement because the condition can no longer occur. +### Seven flow-node config key aliases graduate into the conversion layer (#3796) + +| Node type(s) | Deprecated | Use instead | +|---|---|---| +| `get_record` / `create_record` / `update_record` / `delete_record` | `config.object` | `config.objectName` | +| `notify` | `config.to` / `config.subject` / `config.body` / `config.url` | `config.recipients` / `config.title` / `config.message` / `config.actionUrl` | +| `script` | `config.functionName` / `config.input` | `config.function` / `config.inputs` | + +All are pure key renames with unchanged values. Unlike the three tombstoned +aliases above, these **cannot** be rejected in the schema — +`FlowNodeSchema.config` is an unconstrained record — so they keep a load-path +acceptance window instead: a stored flow authored with an alias keeps loading +through protocol 17, rewritten to the canonical key at load (including +`AutomationEngine.registerFlow` rehydration) with a `ConversionNotice`; the +window retires in 18. The executors read canonical keys only, and the +`readAliasedConfig` executor shim (which covered `object` → `objectName` and +warned at run time) is deleted — the conversion layer is now the single seam +that declares, converts, and retires flow-config aliases. + +`os migrate meta --from ` rewrites all seven in your +source; or rename the keys by hand. `actionUrl` is the deliberate canonical of +its pair: the downstream chain already uses that name +(`sys_notification.action_url`, the channel contract, the REST notification +model), while `url` platform-wide means an HTTP endpoint to call (`http` node, +webhooks). The singular `input` on `map` / `subflow` / `connector_action` is +those nodes' own canonical key and is untouched. + ### `agent.tools[]` is removed — capability comes from skills (ADR-0109, #3820) ADR-0064's invariant is "an agent's tool set is the union of its diff --git a/docs/protocol-upgrade-guide.md b/docs/protocol-upgrade-guide.md index 5882299793..a90db55e64 100644 --- a/docs/protocol-upgrade-guide.md +++ b/docs/protocol-upgrade-guide.md @@ -122,6 +122,8 @@ It also removes the sharing-rule access level `full` (#3865): declared as "Full Finally it removes agent `tools` (#3894): the legacy inline `{type,name,description}[]` fallback, which the runtime resolved against the FULL tool registry with no surface check — the one seam that broke ADR-0064's "an agent reaches exactly its surface-compatible skills' tools, nothing falls through to the global registry". Unlike the renames above this has NO lossless target: each entry has to become a reference inside a skill, which is a human decision about which skill. The conversion therefore drops the dead key (the runtime stopped reading it in cloud#910, so it already contributes nothing) and emits a notice per agent so the author knows where capability must be re-declared; the schema tombstones the key with a fix-it error naming `skills`. +Beyond those spec-surface removals, it graduates the seven flow-node config key aliases the executors still tolerated (#3796): the CRUD nodes' `object` (use `objectName`) — the last tenant of the `readAliasedConfig` executor shim, which is deleted with it — plus the six open-coded fallbacks that never went through that shim: notify `to`/`subject`/`body`/`url` (use `recipients`/`title`/`message`/`actionUrl`) and script `functionName`/`input` (use `function`/`inputs`). All are pure key renames with unchanged values and replay losslessly. Like the sharing-rule access level above they keep a load-path acceptance window: none carried a prior deprecation warning, and `FlowNodeSchema.config` is an unconstrained record, so no schema tombstone can reject them — the conversion layer is the only seam that can declare, convert, and retire them. + ### Mechanical (applied for you) | Conversion | Surface | Change | Load window | @@ -131,6 +133,9 @@ Finally it removes agent `tools` (#3894): the legacy inline `{type,name,descript | `agent-knowledge-topics-to-sources` | `agent.knowledge.topics` | agent knowledge key 'topics' → 'sources' (the deprecated RAG-source alias, #1891) | retired — `migrate meta` only | | `agent-tools-to-skills` | `agent.tools` | agent key 'tools' removed — declare capability in a skill (ADR-0064, #3894) | retired — `migrate meta` only | | `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 | +| `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 | +| `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 | +| `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 | --- diff --git a/packages/lint/src/validate-expressions.ts b/packages/lint/src/validate-expressions.ts index 20dfc3f95b..67b1f39afa 100644 --- a/packages/lint/src/validate-expressions.ts +++ b/packages/lint/src/validate-expressions.ts @@ -130,7 +130,9 @@ export function validateStackExpressions(stack: AnyRec): ExprIssue[] { // not serialized into the artifact — so this is a structural check; the // runtime verifies the named function is actually registered.) if (node.type === 'script') { - // `function` is canonical; `functionName` is an accepted alias. + // `function` is canonical; a pre-parse source may still carry the + // `functionName` alias during the protocol-17 window, until the + // 'flow-node-script-config-aliases' conversion (#3796) canonicalizes it. const fn = (typeof cfg.function === 'string' ? cfg.function.trim() : '') || (typeof cfg.functionName === 'string' ? cfg.functionName.trim() : ''); diff --git a/packages/lint/src/validate-readonly-flow-writes.ts b/packages/lint/src/validate-readonly-flow-writes.ts index 284af14e3e..24052ddb31 100644 --- a/packages/lint/src/validate-readonly-flow-writes.ts +++ b/packages/lint/src/validate-readonly-flow-writes.ts @@ -103,9 +103,11 @@ function buildReadonlyIndex(objects: AnyRec[]): Map(); - -/** 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 index e3d2e9739c..5716e84afa 100644 --- a/packages/services/service-automation/src/builtin/crud-config-aliases.test.ts +++ b/packages/services/service-automation/src/builtin/crud-config-aliases.test.ts @@ -3,25 +3,26 @@ /** * Deprecation window for non-canonical `config` keys on the CRUD nodes. * - * Two aliases, now handled at two different layers: - * - `object` → `objectName` is still tolerated by the **executor** shim - * (`readAliasedConfig`), warning once per alias. See config-aliases.ts. - * - `filters` → `filter` has been **retired from the executor** and promoted - * into the ADR-0087 D2 conversion layer (`@objectstack/spec` conversion - * `flow-node-crud-filter-alias`): it is rewritten to `filter` at load, so a - * raw `filters` key reaching the executor directly (a flow that skipped the - * load seam) is no longer honored, and the executor emits no alias warning - * for it. This test documents that split — the PD #12 retirement path. + * Both aliases are now handled at ONE layer — the ADR-0087 D2 conversion layer: + * - `filters` → `filter` (`flow-node-crud-filter-alias`), and + * - `object` → `objectName` (`flow-node-crud-object-alias`, #3796) — the last + * tenant of the `readAliasedConfig` executor shim, deleted with its + * graduation. + * + * Stored flows are canonicalized on rehydration (`AutomationEngine.registerFlow` + * runs the conversion before parse), so the executors read the canonical keys + * directly and a raw alias reaching an executor that skipped the load seam is + * no longer honored. This test documents that endpoint of the PD #12 + * retirement path. * * Each run below passes a `userId`: a data-touching flow left at the spec * default `runAs:'user'` is refused without a trigger user (#3760). Incidental * to what these tests assert — supplied so the run reaches the CRUD node. */ -import { describe, it, expect, beforeEach } from 'vitest'; +import { describe, it, expect } from 'vitest'; import { normalizeStackInput } from '@objectstack/spec'; 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() {} }; @@ -29,12 +30,6 @@ function silentLogger(): any { 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 = { @@ -67,16 +62,14 @@ function getRecordFlow(config: Record) { } as any; } -describe('CRUD config-key aliases: object→objectName (executor shim) + filters→filter (retired to load-time conversion)', () => { - beforeEach(() => __resetAliasDeprecationWarnings()); - - it('still resolves the deprecated `object` alias at runtime, warning once', async () => { +describe('CRUD config-key aliases: object→objectName + filters→filter (load-time conversions)', () => { + it('a stored `object` flow is canonicalized at registerFlow, so the executor resolves the target', async () => { + // The `object` alias graduation (#3796): what the executor shim used to + // tolerate at run time is now rewritten on rehydration, before parse. const engine = new AutomationEngine(silentLogger()); const { data, calls } = fakeData(); - const warns: string[] = []; - registerCrudNodes(engine, ctxWith(data, collectingLogger(warns))); + registerCrudNodes(engine, ctxWith(data, silentLogger())); - // Canonical `filter`; deprecated `object`. engine.registerFlow('gr', getRecordFlow({ object: 'crm_lead', filter: { id: 'L1' }, outputVariable: 'lead' })); const res = await engine.execute('gr', { userId: 'u1' }); @@ -84,14 +77,6 @@ describe('CRUD config-key aliases: object→objectName (executor shim) + filters expect(calls).toHaveLength(1); expect(calls[0].obj).toBe('crm_lead'); expect(calls[0].opts.where).toEqual({ id: 'L1' }); - - const objectWarn = warns.find((w) => w.includes("'object'") && w.includes("'objectName'")); - expect(objectWarn).toBeTruthy(); - - // One-time per alias: a second run does not warn again. - const before = warns.length; - await engine.execute('gr', { userId: 'u1' }); - expect(warns.length).toBe(before); }); it('a stored `filters` flow is canonicalized at registerFlow, so the filter is NOT dropped', async () => { @@ -111,29 +96,30 @@ describe('CRUD config-key aliases: object→objectName (executor shim) + filters expect(calls[0].opts.where).toEqual({ id: 'L1' }); // filter preserved, not dropped }); - it('the same conversion runs on the build/validate seam too (normalizeStackInput)', async () => { + it('the same conversions run on the build/validate seam too (normalizeStackInput)', async () => { const engine = new AutomationEngine(silentLogger()); const { data, calls } = fakeData(); registerCrudNodes(engine, ctxWith(data, silentLogger())); - const raw = getRecordFlow({ objectName: 'crm_lead', filters: { id: 'L1' }, outputVariable: 'lead' }); + const raw = getRecordFlow({ object: 'crm_lead', filters: { id: 'L1' }, outputVariable: 'lead' }); const converted = (normalizeStackInput({ flows: [raw] }).flows as any[])[0]; engine.registerFlow('gr', converted); await engine.execute('gr', { userId: 'u1' }); + expect(calls[0].obj).toBe('crm_lead'); expect(calls[0].opts.where).toEqual({ id: 'L1' }); }); - it('does NOT warn when the canonical keys are used', async () => { + it('canonical keys pass through the load seam unchanged', async () => { const engine = new AutomationEngine(silentLogger()); - const { data } = fakeData(); - const warns: string[] = []; - registerCrudNodes(engine, ctxWith(data, collectingLogger(warns))); + const { data, calls } = fakeData(); + registerCrudNodes(engine, ctxWith(data, silentLogger())); engine.registerFlow('gr', getRecordFlow({ objectName: 'crm_lead', filter: { id: 'L1' }, outputVariable: 'lead' })); const res = await engine.execute('gr', { userId: 'u1' }); expect(res.success).toBe(true); - expect(warns).toHaveLength(0); + expect(calls[0].obj).toBe('crm_lead'); + expect(calls[0].opts.where).toEqual({ id: 'L1' }); }); }); diff --git a/packages/services/service-automation/src/builtin/crud-nodes.ts b/packages/services/service-automation/src/builtin/crud-nodes.ts index 8abc09eb5d..9826c72d09 100644 --- a/packages/services/service-automation/src/builtin/crud-nodes.ts +++ b/packages/services/service-automation/src/builtin/crud-nodes.ts @@ -6,7 +6,6 @@ import type { IDataEngine } from '@objectstack/spec/contracts'; import type { DroppedFieldsEvent } from '@objectstack/spec/data'; import type { AutomationEngine } from '../engine.js'; import { interpolate, interpolateFilter, type VariableMap } from './template.js'; -import { readAliasedConfig } from './config-aliases.js'; import { refuseNode } from '../guard-refusal.js'; import { resolveRunDataContext } from '../runtime-identity.js'; @@ -161,12 +160,13 @@ export function registerCrudNodes(engine: AutomationEngine, ctx: PluginContext): }), async execute(node, variables, context) { const cfg = (node.config ?? {}) as Record; - const objectName = String(readAliasedConfig(cfg, 'get_record', 'objectName', ['object'], ctx.logger) ?? ''); + // `filters` → `filter` and `object` → `objectName` are handled at + // load by the ADR-0087 D2 conversion layer ('flow-node-crud-filter-alias', + // 'flow-node-crud-object-alias'), so the executor reads the canonical + // keys directly (PD #12 fallbacks retired). + const objectName = String(cfg.objectName ?? ''); if (!objectName) return refuseNode('get_record: objectName required'); - // `filters` → `filter` is now handled at load by the ADR-0087 D2 - // conversion layer ('flow-node-crud-filter-alias'), so the executor - // reads the canonical key directly (PD #12 fallback retired). const filterResult = resolveNodeFilter( cfg.filter, variables, context, 'get_record', 'would have read rows the filter was written to exclude', @@ -222,7 +222,7 @@ export function registerCrudNodes(engine: AutomationEngine, ctx: PluginContext): }), async execute(node, variables, context) { const cfg = (node.config ?? {}) as Record; - const objectName = String(readAliasedConfig(cfg, 'create_record', 'objectName', ['object'], ctx.logger) ?? ''); + const objectName = String(cfg.objectName ?? ''); if (!objectName) return refuseNode('create_record: objectName required'); const fields = interpolate(cfg.fields ?? {}, variables, context) as Record; @@ -303,7 +303,7 @@ export function registerCrudNodes(engine: AutomationEngine, ctx: PluginContext): }), async execute(node, variables, context) { const cfg = (node.config ?? {}) as Record; - const objectName = String(readAliasedConfig(cfg, 'update_record', 'objectName', ['object'], ctx.logger) ?? ''); + const objectName = String(cfg.objectName ?? ''); if (!objectName) return refuseNode('update_record: objectName required'); // `filters` → `filter` converted at load (ADR-0087 D2); read canonical. @@ -376,7 +376,7 @@ export function registerCrudNodes(engine: AutomationEngine, ctx: PluginContext): }), async execute(node, variables, context) { const cfg = (node.config ?? {}) as Record; - const objectName = String(readAliasedConfig(cfg, 'delete_record', 'objectName', ['object'], ctx.logger) ?? ''); + const objectName = String(cfg.objectName ?? ''); if (!objectName) return refuseNode('delete_record: objectName required'); // `filters` → `filter` converted at load (ADR-0087 D2); read canonical. diff --git a/packages/services/service-automation/src/builtin/notify-node.test.ts b/packages/services/service-automation/src/builtin/notify-node.test.ts index fa7fdd3464..2e4c5980b6 100644 --- a/packages/services/service-automation/src/builtin/notify-node.test.ts +++ b/packages/services/service-automation/src/builtin/notify-node.test.ts @@ -169,7 +169,12 @@ describe('notify (baseline node)', () => { expect(messaging.emitted[0].source).toBeUndefined(); }); - it('accepts `url` as an alias for actionUrl', async () => { + // The deprecated aliases (`to`/`subject`/`body`/`url`) are rewritten to + // the canonical keys on rehydration — `registerFlow` runs the ADR-0087 + // D2 conversion 'flow-node-notify-config-aliases' (#3796) — so a stored + // pre-protocol-17 flow keeps working while the executor reads canonical + // keys only. + it('canonicalizes a stored `url` key to `actionUrl` at load', async () => { engine.registerFlow('notify_flow', notifyFlow({ recipients: ['user_1'], title: 'Heads up', @@ -183,7 +188,7 @@ describe('notify (baseline node)', () => { expect(messaging.emitted[0].payload).toMatchObject({ url: '/opps/7' }); }); - it('accepts a single recipient string and the subject/to aliases', async () => { + it('canonicalizes stored `to`/`subject` keys at load (single recipient string form)', async () => { engine.registerFlow('notify_flow', notifyFlow({ to: 'user_9', subject: 'Heads up', diff --git a/packages/services/service-automation/src/builtin/notify-node.ts b/packages/services/service-automation/src/builtin/notify-node.ts index 924e133ef3..3bf47d5b38 100644 --- a/packages/services/service-automation/src/builtin/notify-node.ts +++ b/packages/services/service-automation/src/builtin/notify-node.ts @@ -134,16 +134,18 @@ export function registerNotifyNode(engine: AutomationEngine, ctx: PluginContext) // are still tolerated (JSON Schema allows additional properties) — // this is discoverability, not a lockdown. configSchema: { - // No `required` array: `recipients`/`title` each accept an alias - // (`to`/`subject`), which a strict required-check would reject. - // The node enforces "title + ≥1 recipient" at execute time. + // No `required` array: the execute-time guard ("title + ≥1 + // recipient") owns enforcement, and a strict required-check on a + // RAW stored config would reject a pre-protocol-17 flow whose + // aliased keys (`to`/`subject`/`body`/`url`) are only rewritten + // at the load seam ('flow-node-notify-config-aliases', #3796). type: 'object', properties: { recipients: { - description: 'Recipient user id(s) / audience selector(s); alias: `to`', + description: 'Recipient user id(s) / audience selector(s)', }, - title: { type: 'string', description: 'Notification title; alias: `subject`' }, - message: { type: 'string', description: 'Notification body; alias: `body`' }, + title: { type: 'string', description: 'Notification title' }, + message: { type: 'string', description: 'Notification body' }, channels: { type: 'array', items: { type: 'string' }, description: 'Channels to fan out to (default: inbox)', @@ -163,9 +165,13 @@ export function registerNotifyNode(engine: AutomationEngine, ctx: PluginContext) type: 'string', description: 'User id that caused the event (writes sys_notification.actor_id)', }, - url: { + // `actionUrl` (not `url`) is the deliberate canonical: it is the + // name the whole downstream chain uses (sys_notification.action_url, + // the channel contract, the REST read model), while `url` elsewhere + // in the platform means "HTTP endpoint to call" (#3796). + actionUrl: { type: 'string', - description: 'Explicit click-through URL; overrides the link synthesized from sourceObject/sourceId. Alias: `actionUrl`.', + description: 'Explicit click-through URL; overrides the link synthesized from sourceObject/sourceId.', }, payload: { type: 'object', description: 'Extra template inputs merged into the notification payload' }, }, @@ -174,17 +180,20 @@ export function registerNotifyNode(engine: AutomationEngine, ctx: PluginContext) async execute(node, variables, context) { const cfg = (node.config ?? {}) as Record; - const recipientCfg = cfg.recipients ?? cfg.to ?? []; + // The historical aliases (`to`/`subject`/`body`/`url`) are canonicalized + // at load by the ADR-0087 D2 conversion 'flow-node-notify-config-aliases' + // (#3796), so only the canonical keys are read here. + const recipientCfg = cfg.recipients ?? []; const recipients = toStringList(interpolate(recipientCfg, variables, context)); // stringifyForTemplate (not String()): a sole-token `{$error}` resolves // to the engine's error OBJECT, which String() would render as the // useless `[object Object]` (#3450). Serialize it readably instead. - const title = stringifyForTemplate(interpolate(cfg.title ?? cfg.subject ?? '', variables, context)); - const body = stringifyForTemplate(interpolate(cfg.message ?? cfg.body ?? '', variables, context)); + const title = stringifyForTemplate(interpolate(cfg.title ?? '', variables, context)); + const body = stringifyForTemplate(interpolate(cfg.message ?? '', variables, context)); const channels = toStringList(cfg.channels); const topic = cfg.topic ? String(cfg.topic) : undefined; const severity = cfg.severity ? String(cfg.severity) : undefined; - const urlCfg = cfg.actionUrl ?? cfg.url; + const urlCfg = cfg.actionUrl; const actionUrl = urlCfg ? String(interpolate(urlCfg, variables, context) ?? '') : undefined; @@ -195,11 +204,11 @@ export function registerNotifyNode(engine: AutomationEngine, ctx: PluginContext) // Click-through target: forwarding `source` lets the messaging // service persist sys_notification.source_object/source_id and // synthesize a `/{object}/{id}` deep-link for the inbox (#2675). An - // explicit `actionUrl`/`url` still wins over the synthesized link. + // explicit `actionUrl` still wins over the synthesized link. const source = resolveSource(cfg, variables, context); const actorId = toStr(interpolate(cfg.actorId, variables, context)); - if (!title) return { success: false, error: 'notify: title (or subject) is required' }; + if (!title) return { success: false, error: 'notify: title is required' }; if (recipients.length === 0) { // Name the templates that came up empty (framework#3582). The // dominant cause is a cross-object hop — `{record.owner.manager}` diff --git a/packages/services/service-automation/src/builtin/screen-nodes.test.ts b/packages/services/service-automation/src/builtin/screen-nodes.test.ts index 6be603fb22..3a9fe4c59d 100644 --- a/packages/services/service-automation/src/builtin/screen-nodes.test.ts +++ b/packages/services/service-automation/src/builtin/screen-nodes.test.ts @@ -107,7 +107,10 @@ describe('script node (#1870 — callable resolution)', () => { expect(result.success).toBe(false); expect(result.error).toMatch(/explode.*failed|failed.*boom|boom/i); }); -it('resolves config.functionName as an alias for function (#1870 DX)', async () => { +it('canonicalizes a stored `functionName` key to `function` at load (#1870 DX, #3796)', async () => { + // `registerFlow` runs the ADR-0087 D2 conversion + // 'flow-node-script-config-aliases', so the deprecated alias keeps + // working for stored flows while the executor reads `function` only. let calledWith: any; engine.setFunctionResolver((name) => name === 'helpdesk.aiTriageStub' ? ((c: any) => { calledWith = c.input; return { triaged: true }; }) : undefined); @@ -118,7 +121,7 @@ it('resolves config.functionName as an alias for function (#1870 DX)', async () }); it('treats actionType invoke_function as a marker, not a function name', async () => { - // invoke_function alone (no function/functionName) must NOT try to resolve a + // invoke_function alone (no `function`) must NOT try to resolve a // function literally named 'invoke_function'; it fails with a clear message. engine.registerFlow('script_flow', scriptFlow({ actionType: 'invoke_function' })); const r = await engine.execute('script_flow', {} as any); diff --git a/packages/services/service-automation/src/builtin/screen-nodes.ts b/packages/services/service-automation/src/builtin/screen-nodes.ts index cb9942240d..13a164f42f 100644 --- a/packages/services/service-automation/src/builtin/screen-nodes.ts +++ b/packages/services/service-automation/src/builtin/screen-nodes.ts @@ -175,9 +175,10 @@ export function registerScreenNodes(engine: AutomationEngine, ctx: PluginContext }), async execute(node, variables, context) { const cfg = (node.config ?? {}) as Record; - // `function` is canonical; `functionName` is an accepted alias — AI/templates - // commonly emit it alongside `actionType: 'invoke_function'` (#1870 DX). - const fnRaw = cfg.function ?? cfg.functionName; + // The historical aliases (`functionName`/`input`) are canonicalized at + // load by the ADR-0087 D2 conversion 'flow-node-script-config-aliases' + // (#3796), so only the canonical keys are read here. + const fnRaw = cfg.function; const fnName = typeof fnRaw === 'string' && fnRaw.trim() ? fnRaw.trim() : undefined; const actionType = typeof cfg.actionType === 'string' && cfg.actionType.trim() ? cfg.actionType.trim() : undefined; @@ -212,8 +213,8 @@ export function registerScreenNodes(engine: AutomationEngine, ctx: PluginContext } // `actionType: 'invoke_function'` is a MARKER meaning "call the named - // function" — the name lives in `function`/`functionName`, not in actionType - // itself. A bare actionType that matched no built-in is still accepted as a + // function" — the name lives in `function`, not in actionType itself. A + // bare actionType that matched no built-in is still accepted as a // function name (shorthand). const target = fnName ?? (actionType === 'invoke_function' ? undefined : actionType); if (!target) { @@ -221,7 +222,7 @@ export function registerScreenNodes(engine: AutomationEngine, ctx: PluginContext success: false, error: actionType === 'invoke_function' - ? `script node '${node.id}': actionType 'invoke_function' requires \`config.function\` (or \`functionName\`) naming the function to call.` + ? `script node '${node.id}': actionType 'invoke_function' requires \`config.function\` naming the function to call.` : `script node '${node.id}': declares neither \`actionType\` nor \`function\` — nothing to run.`, }; } @@ -237,10 +238,10 @@ export function registerScreenNodes(engine: AutomationEngine, ctx: PluginContext }; } - // Map declared inputs (`config.inputs` | `config.input`) to the function, - // interpolating `{var}` references against the live flow variables (so a - // function can consume a prior node's output, e.g. `{aiResult.id}`). - const input = interpolate(cfg.inputs ?? cfg.input ?? {}, variables, context) as Record; + // Map declared inputs (`config.inputs`) to the function, interpolating + // `{var}` references against the live flow variables (so a function can + // consume a prior node's output, e.g. `{aiResult.id}`). + const input = interpolate(cfg.inputs ?? {}, variables, context) as Record; const outputVariable = typeof cfg.outputVariable === 'string' && cfg.outputVariable.trim() ? cfg.outputVariable.trim() : undefined; try { diff --git a/packages/spec/spec-changes.json b/packages/spec/spec-changes.json index 9b02c11174..9ce8ea6df3 100644 --- a/packages/spec/spec-changes.json +++ b/packages/spec/spec-changes.json @@ -97,6 +97,24 @@ "to": "sharing-rule accessLevel 'full' → 'edit' (#3865 — `full` never granted more than `edit`)", "conversionId": "sharing-rule-access-level-full-to-edit", "toMajor": 17 + }, + { + "surface": "flow.node.config.objectName", + "to": "CRUD flow-node config key 'object' → 'objectName' (#3796 — `readAliasedConfig` shim graduation)", + "conversionId": "flow-node-crud-object-alias", + "toMajor": 17 + }, + { + "surface": "flow.node.notify.config", + "to": "notify flow-node config keys 'to' → 'recipients', 'subject' → 'title', 'body' → 'message', 'url' → 'actionUrl' (#3796)", + "conversionId": "flow-node-notify-config-aliases", + "toMajor": 17 + }, + { + "surface": "flow.node.script.config", + "to": "script flow-node config keys 'functionName' → 'function', 'input' → 'inputs' (#3796)", + "conversionId": "flow-node-script-config-aliases", + "toMajor": 17 } ], "migrated": [ @@ -395,6 +413,24 @@ "to": "sharing-rule accessLevel 'full' → 'edit' (#3865 — `full` never granted more than `edit`)", "conversionId": "sharing-rule-access-level-full-to-edit", "toMajor": 17 + }, + { + "surface": "flow.node.config.objectName", + "to": "CRUD flow-node config key 'object' → 'objectName' (#3796 — `readAliasedConfig` shim graduation)", + "conversionId": "flow-node-crud-object-alias", + "toMajor": 17 + }, + { + "surface": "flow.node.notify.config", + "to": "notify flow-node config keys 'to' → 'recipients', 'subject' → 'title', 'body' → 'message', 'url' → 'actionUrl' (#3796)", + "conversionId": "flow-node-notify-config-aliases", + "toMajor": 17 + }, + { + "surface": "flow.node.script.config", + "to": "script flow-node config keys 'functionName' → 'function', 'input' → 'inputs' (#3796)", + "conversionId": "flow-node-script-config-aliases", + "toMajor": 17 } ], "migrated": [], diff --git a/packages/spec/src/conversions/registry.ts b/packages/spec/src/conversions/registry.ts index 96d005c88a..9a131974d1 100644 --- a/packages/spec/src/conversions/registry.ts +++ b/packages/spec/src/conversions/registry.ts @@ -887,6 +887,231 @@ const sharingRuleAccessLevelFullToEdit: MetadataConversion = { }, }; +/** Rename each `[from, to]` config pair on flow nodes of the given types. */ +function renameFlowConfigAliases( + stack: Dict, + nodeTypes: ReadonlySet, + pairs: ReadonlyArray, + emit: Emit, +): Dict { + return mapFlowNodes(stack, (node, path) => { + if (typeof node.type !== 'string' || !nodeTypes.has(node.type)) return node; + let next = node; + for (const [from, to] of pairs) { + const renamed = renameConfigKey(next, from, to); + if (!renamed) continue; + emit({ from, to, path: `${path}.config.${to}` }); + next = renamed; + } + return next; + }); +} + +/** + * CRUD flow-node `config.object` → `config.objectName` (protocol 17, #3796). + * + * The last tenant of the `readAliasedConfig` executor shim + * (`service-automation/src/builtin/config-aliases.ts`) graduates into the + * conversion layer, completing the PD #12 retirement path that + * {@link flowNodeFilterAlias} pioneered: the alias is rewritten to the + * canonical key at load — including the `AutomationEngine.registerFlow` + * rehydration seam — so the CRUD executors read `cfg.objectName` directly and + * the shim is deleted. **Live window**: stored flows authored with `object` + * keep loading through this major; retires at 18. + */ +const flowNodeCrudObjectAlias: MetadataConversion = { + id: 'flow-node-crud-object-alias', + toMajor: 17, + surface: 'flow.node.config.objectName', + summary: "CRUD flow-node config key 'object' → 'objectName' (#3796 — `readAliasedConfig` shim graduation)", + apply(stack, emit) { + const crudTypes = new Set(['get_record', 'create_record', 'update_record', 'delete_record']); + return renameFlowConfigAliases(stack, crudTypes, [['object', 'objectName']], emit); + }, + fixture: { + before: { + flows: [ + { + name: 'lead_lookup', + nodes: [ + { id: 'n1', type: 'start' }, + { id: 'n2', type: 'get_record', config: { object: 'lead', recordId: '{leadId}' } }, + // canonical already present → the shadowed alias is left alone (no notice) + { id: 'n3', type: 'create_record', config: { objectName: 'task', object: 'ignored' } }, + ], + }, + ], + }, + after: { + flows: [ + { + name: 'lead_lookup', + nodes: [ + { id: 'n1', type: 'start' }, + { id: 'n2', type: 'get_record', config: { objectName: 'lead', recordId: '{leadId}' } }, + { id: 'n3', type: 'create_record', config: { objectName: 'task', object: 'ignored' } }, + ], + }, + ], + }, + expectedNotices: 1, + }, +}; + +/** + * Notify flow-node config key aliases → canonical (protocol 17, #3796). + * + * The `notify` executor carried four open-coded `??` fallbacks that never went + * through the deprecation shim — an author who wrote the email-idiom keys got + * a flow that worked forever and was never steered to the canonical spelling. + * All four are pure key renames with unchanged values. + * + * `actionUrl` is the deliberate canonical of its pair (the executor's own + * `configSchema` used to claim the opposite): the entire downstream chain + * already uses it — `sys_notification.action_url`, the channel-dispatch + * contract, the REST notification read model — and `url` elsewhere in the + * platform means "HTTP endpoint to call" (`http` node, webhooks), a different + * concept from this in-app click-through target. The executor precedence + * already put `actionUrl` first, so the choice is behaviour-preserving. + * **Live window**; retires at 18. + */ +const flowNodeNotifyConfigAliases: MetadataConversion = { + id: 'flow-node-notify-config-aliases', + toMajor: 17, + surface: 'flow.node.notify.config', + summary: + "notify flow-node config keys 'to' → 'recipients', 'subject' → 'title', 'body' → 'message', 'url' → 'actionUrl' (#3796)", + apply(stack, emit) { + return renameFlowConfigAliases( + stack, + new Set(['notify']), + [ + ['to', 'recipients'], + ['subject', 'title'], + ['body', 'message'], + ['url', 'actionUrl'], + ], + emit, + ); + }, + fixture: { + before: { + flows: [ + { + name: 'task_assigned', + nodes: [ + { id: 'n1', type: 'start' }, + { + id: 'n2', + type: 'notify', + config: { + to: ['{record.assignee}'], + subject: 'New task: {record.title}', + body: 'You have been assigned "{record.title}".', + url: '/task/{record.id}', + channels: ['inbox'], + }, + }, + ], + }, + ], + }, + after: { + flows: [ + { + name: 'task_assigned', + nodes: [ + { id: 'n1', type: 'start' }, + { + id: 'n2', + type: 'notify', + config: { + recipients: ['{record.assignee}'], + title: 'New task: {record.title}', + message: 'You have been assigned "{record.title}".', + actionUrl: '/task/{record.id}', + channels: ['inbox'], + }, + }, + ], + }, + ], + }, + expectedNotices: 4, + }, +}; + +/** + * Script flow-node config key aliases → canonical (protocol 17, #3796). + * + * `function` is the canonical callable reference (#1870); `functionName` was + * the AI/template-emitted alias. `inputs` is the canonical input map; the + * `input` alias almost certainly leaked from `connector_action`, whose + * `connectorConfig.input` (singular) is a *different, canonical* surface and is + * deliberately not touched here. Both are pure key renames with unchanged + * values. **Live window**; retires at 18. + */ +const flowNodeScriptConfigAliases: MetadataConversion = { + id: 'flow-node-script-config-aliases', + toMajor: 17, + surface: 'flow.node.script.config', + summary: "script flow-node config keys 'functionName' → 'function', 'input' → 'inputs' (#3796)", + apply(stack, emit) { + return renameFlowConfigAliases( + stack, + new Set(['script']), + [ + ['functionName', 'function'], + ['input', 'inputs'], + ], + emit, + ); + }, + fixture: { + before: { + flows: [ + { + name: 'score_lead', + nodes: [ + { id: 'n1', type: 'start' }, + { + id: 'n2', + type: 'script', + config: { + actionType: 'invoke_function', + functionName: 'score_lead', + input: { leadId: '{record.id}' }, + outputVariable: 'score', + }, + }, + ], + }, + ], + }, + after: { + flows: [ + { + name: 'score_lead', + nodes: [ + { id: 'n1', type: 'start' }, + { + id: 'n2', + type: 'script', + config: { + actionType: 'invoke_function', + function: 'score_lead', + inputs: { leadId: '{record.id}' }, + outputVariable: 'score', + }, + }, + ], + }, + ], + }, + expectedNotices: 2, + }, +}; + /** * All conversions, keyed by the protocol major that introduced the canonical * shape. Newest majors last; ordering within a major is application order. @@ -902,6 +1127,9 @@ export const CONVERSIONS_BY_MAJOR: Readonly