diff --git a/.changeset/22642-flow-cel-record-binding.md b/.changeset/22642-flow-cel-record-binding.md new file mode 100644 index 00000000000..4cadfa39a9d --- /dev/null +++ b/.changeset/22642-flow-cel-record-binding.md @@ -0,0 +1,38 @@ +--- +'@objectstack/service-automation': minor +--- + +fix(service-automation)!: in flow CEL, `record` is the record the run was handed, or unbound — never the run's variables + +Clause-②: no (narrowing) + + + +**BREAKING**, graded `minor` on the v18 prerelease line: Changesets is in pre mode with the tag `next`, and the fixed group is already majored by the line's opening marker, so this ships in an `18.0.0-next.N`. + +**What changed.** A flow CEL expression — a node's or an edge's `condition`, a `decision` branch `expression`, a screen field's `visibleWhen`, and the CEL value envelopes of an `assignment` and of a `create_record` / `update_record` `fields` map — evaluates in the scope `AutomationEngine.celScope` builds. That scope used to bind `record` to the run's variables map whenever the run held no record, so `record.assignee` read a flow variable named `assignee` instead of failing. Now `record` is bound only when: + +- an entrance handed the run a record: a record-change trigger's row, a time-relative sweep's row, the inbound hook's request body, a `type: 'flow'` action's record (the loaded row, or an empty record carrying at most the id it was given), a `subflow` or `map` parent's record, or a `map` item that carries a string `id`; +- or the flow binds a variable named `record` itself (a declared variable with a value, or an assignment target). + +With neither — a run started through the REST trigger route or a declared endpoint, a cron schedule, or a child of such a run — `record.X` fails the run with `Unknown variable: record` and the expression's source, as every other unbound root does. + +**What does not change.** A run that was handed a record reads `record.X` from that record, as before, over any variable of the same name. A bare variable name (`assignee`) and the `vars` namespace (`vars.assignee`) resolve as before. `$record`, `previous`, `vars` and `current_user` are untouched. + +## Migration: FROM → TO + +In a flow whose run holds no record, read a variable by its name: + +| you wrote | write instead | +|:--|:--| +| `record.assignee` (a flow variable `assignee`) | `assignee`, or `vars.assignee` | +| `has(record.assignee) ? record.assignee : null` | `has(vars.assignee) ? vars.assignee : null` | + +**The one-line fix: read a flow variable by its name or through `vars`; `record` is only ever the record the run was handed.** A flow that is also launched with a record (an action on a row, a record trigger) keeps reading that record's fields as `record.X`. + +**Who is affected, measured.** Read statically over every flow CEL slot — 0 flows read `record` with no record entrance: + +- this repository at `0ec4268972` (`examples/**`, `packages/platform-objects`, `packages/qa/dogfood`): 64 flows, 51 CEL slots, no flow in `packages/platform-objects`; the 2 flows that read `record` are an `api` hook flow and a record-change flow; +- `objectstack-ai/hotcrm` at `f0afcbda07` (`src/`, `test/`): 45 flows, 57 CEL slots; the 12 flows that read `record` are all record-change flows. + +Deployed metadata and other repositories were not measured. `objectstack validate` does not refuse such a `record` read yet; the run fails it, naming the root and the source. diff --git a/packages/services/service-automation/src/engine.ts b/packages/services/service-automation/src/engine.ts index 3e00d5ca260..69bb14b848c 100644 --- a/packages/services/service-automation/src/engine.ts +++ b/packages/services/service-automation/src/engine.ts @@ -12097,11 +12097,40 @@ export class AutomationEngine implements IAutomationService { * each wins over a flow variable of the same name — which is then read as * `vars["vars"]` / `vars["current_user"]` (the spec's * `FLOW_SCOPE_CLAIMED_IDENTIFIERS`, measured from here). + * + * ## `record` — the record the run was handed, or unbound (#22642) + * + * This builder binds no `record` of its own. `record` resolves through the + * spread like any other name, so it is bound exactly when the variable map + * holds a `record` key: + * + * - an entrance handed the run a record. {@link seedRunVariables} binds + * `context.record` as `record` (and `$record`), and every entrance hands + * its record there: a record-change trigger's row, a time-relative + * sweep's row, the inbound hook's request body (`trigger-api`), a + * `type: 'flow'` action's record (the row it loaded, or an empty record + * carrying at most the `id` it was given), a parent's (`subflow` and + * `map` spread the parent's context into the child's) and a `map` item + * that carries a string `id`. The REST trigger route, a declared + * endpoint and a cron schedule hand none; + * - or the flow binds a variable named `record` itself: a declared + * variable that has a value, or an assignment target. + * + * With neither, `record.X` faults `Unknown variable: record`, as every + * other unbound root does. It used to be bound to the variables map + * (`record: vars`, the formula engine's `record` slot, which `extra` + * overrides only when a `record` variable exists), so with no record in + * hand `record.assignee` silently read a variable named `assignee`. A + * variable is read by its name (`assignee`) or through `vars` + * (`vars.assignee`). ⛔ Never bind `record` to the variable map again: a + * record root that answers when the run holds no record is a second + * spelling of every variable, and it hides the author's mistake instead + * of naming it. */ private celScope( variables: Map, context?: AutomationContext, - ): { extra: Record; record: Record } { + ): { extra: Record } { const vars: Record = {}; for (const [key, value] of variables) { // Convert "step.result" keys into nested object paths. @@ -12115,7 +12144,9 @@ export class AutomationEngine implements IAutomationService { } cursor[segs[segs.length - 1]] = value; } - return { extra: { ...vars, vars, current_user: runUserOf(context) }, record: vars }; + // [#22642] No `record` slot: `record` is bound by the spread when the + // run holds one, and unbound otherwise (see the docblock). + return { extra: { ...vars, vars, current_user: runUserOf(context) } }; } /** diff --git a/packages/services/service-automation/src/flow-cel-record-binding.test.ts b/packages/services/service-automation/src/flow-cel-record-binding.test.ts new file mode 100644 index 00000000000..c8099a9d3cc --- /dev/null +++ b/packages/services/service-automation/src/flow-cel-record-binding.test.ts @@ -0,0 +1,265 @@ +// Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license. + +/** + * [#22642] Flow CEL `record` is the record the run was handed, or unbound — + * never the run's variables map. + * + * `AutomationEngine.celScope` used to hand the formula engine `record: vars`, + * and `extra` overrides that slot only when a `record` variable exists. So a + * run with no record in hand bound `record` to its own variables, and + * `record.assignee` silently read a variable named `assignee` instead of + * failing as an unbound root. + * + * Triage's direction, pinned here through the public doors (`registerFlow` + + * `execute`) and the two engine primitives the executors call: + * + * - bare variable names resolve through the spread (`extra`) only; + * - `record` is bound only when an entrance handed the run a record + * (`seedRunVariables` binds `context.record` as `record`), or the flow binds + * a `record` variable itself; + * - with neither, `record.X` faults `Unknown variable: record`, as every other + * unbound root does. + * + * Each entrance's context is spelled the way its door builds it: the + * record-change trigger, the time-relative sweep (`trigger-schedule`), the + * inbound hook (`trigger-api`), a `type: 'flow'` action (`dispatchFlowAction` + * in `@objectstack/runtime`, which always hands a record — the loaded row, or + * an empty one carrying at most the id it was given), a `subflow` parent and a + * `map` item. + */ + +import { describe, expect, it } from 'vitest'; + +import { AutomationEngine } from './engine.js'; +import { installBuiltinNodes } from './builtin/index.js'; + +function silentLogger(): any { + return { info() {}, warn() {}, error() {}, debug() {}, child() { return silentLogger(); } }; +} + +function makeEngine(): AutomationEngine { + const engine = new AutomationEngine(silentLogger()); + installBuiltinNodes(engine, { logger: silentLogger(), getService() { return undefined; } } as any); + return engine; +} + +/** + * start → assignment (`v` = the CEL `source`) → end. `assignee` and `title` are + * input variables, so a run can bind a variable of the same name as a record + * field and the pin can tell which one `record.X` read. + */ +function readerFlow(name: string, source: string, variables: unknown[] = []) { + return { + name, label: name, type: 'autolaunched', + variables: [ + { name: 'assignee', type: 'text', isInput: true }, + { name: 'title', type: 'text', isInput: true }, + { name: 'v', type: 'text', isOutput: true }, + ...variables, + ], + nodes: [ + { id: 'start', type: 'start', label: 'Start' }, + { id: 'set', type: 'assignment', label: 'Set', config: { assignments: { v: { dialect: 'cel', source } } } }, + { id: 'end', type: 'end', label: 'End' }, + ], + edges: [ + { id: 'e1', source: 'start', target: 'set' }, + { id: 'e2', source: 'set', target: 'end' }, + ], + } as any; +} + +async function run(source: string, context: Record, variables: unknown[] = []) { + const engine = makeEngine(); + engine.registerFlow('reader', readerFlow('reader', source, variables)); + return engine.execute('reader', context as any); +} + +const outputOf = (result: { output?: unknown }) => (result.output as { v?: unknown } | undefined)?.v; + +/** A run with an `assignee` variable and no record in hand: the REST trigger route's shape. */ +const NO_RECORD = { params: { assignee: 'u9' } }; + +describe('no record in hand: `record` is unbound, never the variables map', () => { + it.each([ + ['record.assignee'], + // The card's own shape: the guard reads the alias as present and answers the variable. + ['has(record.assignee) ? record.assignee : null'], + ])('`%s` fails the run on `record` and never answers the `assignee` variable', async (source) => { + const result = await run(source, NO_RECORD); + expect(result.success).toBe(false); + expect(result.status).toBe('failed'); + expect(result.error).toMatch(/Unknown variable: record\b/); + expect(result.error).toContain(source); + expect(outputOf(result)).not.toBe('u9'); + }); + + it('a predicate site reads the same scope: an edge condition on `record.assignee` fails the run', async () => { + const engine = makeEngine(); + engine.registerFlow('gate', { + name: 'gate', label: 'Gate', type: 'autolaunched', + variables: [{ name: 'assignee', type: 'text', isInput: true }], + nodes: [ + { id: 'start', type: 'start', label: 'Start' }, + { id: 'hit', type: 'end', label: 'Hit' }, + { id: 'miss', type: 'end', label: 'Miss' }, + ], + edges: [ + { id: 'e1', source: 'start', target: 'hit', condition: 'record.assignee == "u9"' }, + { id: 'e2', source: 'start', target: 'miss', condition: 'record.assignee != "u9"' }, + ], + } as any); + const result = await engine.execute('gate', NO_RECORD as any); + expect(result.success).toBe(false); + expect(result.status).toBe('failed'); + expect(result.error).toMatch(/Unknown variable: record\b/); + }); + + it('the engine primitives the executors call refuse it too, naming `record`', () => { + const engine = makeEngine(); + const variables = new Map([['assignee', 'u9']]); + expect(() => engine.evaluateValueEnvelope( + { dialect: 'cel', source: 'has(record.assignee) ? record.assignee : null' }, variables, 'where', + )).toThrow(/Unknown variable: record\b/); + expect(() => engine.evaluateCondition('record.assignee == "u9"', variables)).toThrow(/Unknown variable: record\b/); + }); +}); + +describe('controls: what stays as it was', () => { + it('a record-triggered run reads `record.X` from its record, over a variable of the same name', async () => { + const result = await run('record.assignee', { + record: { id: 'r1', assignee: 'rec_owner' }, + object: 'task', + event: 'record-after-update', + params: { assignee: 'u9' }, + }); + expect(result.success, result.error).toBe(true); + expect(outputOf(result)).toBe('rec_owner'); + }); + + it.each([['assignee'], ['vars.assignee']])('`%s` still reads the variable, with no record in hand', async (source) => { + const result = await run(source, NO_RECORD); + expect(result.success, result.error).toBe(true); + expect(outputOf(result)).toBe('u9'); + }); + + it('a flow that declares a `record` variable reads it, with no entrance record', async () => { + const result = await run('record.assignee', NO_RECORD, [ + { name: 'record', type: 'object', defaultValue: { assignee: 'declared' } }, + ]); + expect(result.success, result.error).toBe(true); + expect(outputOf(result)).toBe('declared'); + }); +}); + +describe('each entrance hands its record: `record` is that record, never the variables', () => { + const ROW = { id: 'r1', title: 'From the record' }; + const VARIABLE = { title: 'From a variable' }; + + it.each([ + ['a record-change trigger', { record: ROW, object: 'task', event: 'record-after-update', previous: { id: 'r1' } }], + ['a time-relative sweep', { record: ROW, object: 'task', event: 'time_relative' }], + ['the inbound hook (the request body is the record)', { record: ROW, params: { ...ROW }, event: 'api' }], + ['a `type: \'flow\'` action on a row', { record: ROW, object: 'task', params: { recordId: 'r1' } }], + ])('%s', async (_entrance, context) => { + const result = await run('record.title', { + ...context, + params: { ...VARIABLE, ...((context as { params?: object }).params ?? {}) }, + }); + expect(result.success, result.error).toBe(true); + expect(outputOf(result)).toBe('From the record'); + }); + + it('an object-less action hands an empty record: `record.X` fails on the key and never reads the variable', async () => { + const result = await run('record.assignee', { record: {}, params: { assignee: 'u9' } }); + expect(result.success).toBe(false); + expect(result.status).toBe('failed'); + expect(result.error).toMatch(/\bassignee\b/); + expect(result.error).not.toMatch(/Unknown variable: record\b/); + expect(outputOf(result)).not.toBe('u9'); + }); + + /** parent: start → subflow (`child`, input `title` = 'From a variable') → end, child output under `sub`. */ + function withSubflow(engine: AutomationEngine) { + engine.registerFlow('child', readerFlow('child', 'record.title')); + engine.registerFlow('parent', { + name: 'parent', label: 'Parent', type: 'autolaunched', + variables: [{ name: 'sub', type: 'object', isOutput: true }], + nodes: [ + { id: 'start', type: 'start', label: 'Start' }, + { + id: 'call', type: 'subflow', label: 'Call', + config: { flowName: 'child', input: { title: 'From a variable' }, outputVariable: 'sub' }, + }, + { id: 'end', type: 'end', label: 'End' }, + ], + edges: [ + { id: 'e1', source: 'start', target: 'call' }, + { id: 'e2', source: 'call', target: 'end' }, + ], + } as any); + } + + it('a `subflow` child reads its parent\'s record', async () => { + const engine = makeEngine(); + withSubflow(engine); + const result = await engine.execute('parent', { record: ROW, object: 'task', event: 'record-after-update' } as any); + expect(result.success, result.error).toBe(true); + expect((result.output as { sub?: { v?: unknown } }).sub?.v).toBe('From the record'); + }); + + it('a parent with no record hands none: the child\'s `record.X` fails, though the child has the variable', async () => { + const engine = makeEngine(); + withSubflow(engine); + const result = await engine.execute('parent', {} as any); + expect(result.success).toBe(false); + expect(result.error).toMatch(/Unknown variable: record\b/); + }); + + /** parent: start → map over `items` (child per item) → end, results under `mapped`. */ + function withMap(engine: AutomationEngine) { + engine.registerFlow('child', readerFlow('child', 'record.title')); + engine.registerFlow('parent', { + name: 'parent', label: 'Parent', type: 'autolaunched', + variables: [ + { name: 'items', type: 'list', isInput: true }, + { name: 'mapped', type: 'list', isOutput: true }, + ], + nodes: [ + { id: 'start', type: 'start', label: 'Start' }, + { + id: 'each', type: 'map', label: 'Each', + config: { + flowName: 'child', collection: '{items}', iteratorVariable: 'item', + input: { title: 'From a variable' }, outputVariable: 'mapped', + }, + }, + { id: 'end', type: 'end', label: 'End' }, + ], + edges: [ + { id: 'e1', source: 'start', target: 'each' }, + { id: 'e2', source: 'each', target: 'end' }, + ], + } as any); + } + + it('a `map` item that carries an id is the child\'s record', async () => { + const engine = makeEngine(); + withMap(engine); + const result = await engine.execute('parent', { + params: { items: [{ id: 'i1', title: 'One' }, { id: 'i2', title: 'Two' }] }, + } as any); + expect(result.success, result.error).toBe(true); + expect((result.output as { mapped?: unknown }).mapped).toEqual([{ v: 'One' }, { v: 'Two' }]); + }); + + it('a `map` item with no id is not a record: the child reads the parent\'s record', async () => { + const engine = makeEngine(); + withMap(engine); + const result = await engine.execute('parent', { + record: ROW, object: 'task', event: 'record-after-update', params: { items: ['a'] }, + } as any); + expect(result.success, result.error).toBe(true); + expect((result.output as { mapped?: unknown }).mapped).toEqual([{ v: 'From the record' }]); + }); +}); diff --git a/packages/spec/src/migrations/entries/semantic/18.flow-cel-record-variables-alias-retired.ts b/packages/spec/src/migrations/entries/semantic/18.flow-cel-record-variables-alias-retired.ts new file mode 100644 index 00000000000..42daf93449d --- /dev/null +++ b/packages/spec/src/migrations/entries/semantic/18.flow-cel-record-variables-alias-retired.ts @@ -0,0 +1,43 @@ +// Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license. + +import type { SemanticMigration } from '../../types.js'; + +// A flow CEL expression's record root is the record the run was handed, or +// unbound. The flow engine used to bind record to the run's own variables +// whenever the run held no record, so record.assignee silently read a flow +// variable named assignee. That spelling now fails the run with an unknown- +// variable fault on record. Semantic-only: whether a run holds a record depends +// on the entrances that start the flow (a trigger, an action, a parent flow, a +// map item), which are not visible from the flow alone, so no D2 conversion can +// rewrite the source mechanically. +export const entry: SemanticMigration = { + id: 'flow-cel-record-variables-alias-retired', + // No backticks in `surface` — build-upgrade-guide renders it inside a code + // span already, and a nested backtick would close it. + surface: + 'flows[].nodes[].config.condition, flows[].edges[].condition, a decision node\'s conditions[].expression, a ' + + 'screen node\'s fields[].visibleWhen, and the CEL value envelopes of an assignment node\'s assignments and of ' + + 'a create_record or update_record node\'s fields — a CEL source reading record.X, or record bare, on a run ' + + 'that holds no record, where X names a flow variable', + replacement: + 'the variable by its name, or through the vars namespace: record.assignee becomes assignee, or vars.assignee. ' + + 'A record.X read on a run that was handed a record is unchanged, and reads that record\'s field X', + reason: + 'Flow CEL binds record to the record the run was handed: a record-change trigger\'s row, a time-relative ' + + 'sweep\'s row, the inbound hook\'s request body, a flow action\'s record (the loaded row, or an empty record ' + + 'carrying at most the id it was given), a subflow or map parent\'s record, or a map item that carries an id. ' + + 'A flow can also bind a variable named record itself. A run with none of these, one started through the REST ' + + 'trigger route, a declared endpoint or a cron schedule, or a child of such a run, used to see record bound to ' + + 'its own variables map, so record.X answered a flow variable named X instead of failing: a second spelling of ' + + 'every variable, which hid the author\'s mistake. Such a run now fails the expression with an unknown-variable ' + + 'fault on record, carrying the source, as every other unbound root does. No D2 conversion exists: whether a ' + + 'run holds a record depends on the entrances that start the flow, which the flow alone does not show, so ' + + 'only the author can tell a variable read from a record field read. Measured at the change: 0 flows in this ' + + 'repository\'s example apps, platform objects and dogfood suites, and 0 in objectstack-ai/hotcrm, read record ' + + 'with no record entrance.', + acceptanceCriteria: + 'For each flow whose CEL reads record, list the entrances that start it. Where any of them hands no record, ' + + 'replace each record.X that names a flow variable with the variable\'s name or vars.X. Re-run each flow path ' + + 'started with no record and confirm every expression evaluates instead of failing with an unknown-variable ' + + 'fault on record; re-run a path started with a record and confirm record.X still reads that record\'s field.', +}; diff --git a/packages/spec/src/migrations/registry.ts b/packages/spec/src/migrations/registry.ts index 356d0e8d803..5fe5f5b5523 100644 --- a/packages/spec/src/migrations/registry.ts +++ b/packages/spec/src/migrations/registry.ts @@ -13793,6 +13793,45 @@ const step18: MigrationStep = { + 'number or boolean slot of any other builtin it fails at its first run exactly as it did, so write a ' + 'literal there.', }, + // A flow CEL expression's record root is the record the run was handed, or + // unbound. The flow engine used to bind record to the run's own variables + // whenever the run held no record, so record.assignee silently read a flow + // variable named assignee. That spelling now fails the run with an unknown- + // variable fault on record. Semantic-only: whether a run holds a record depends + // on the entrances that start the flow (a trigger, an action, a parent flow, a + // map item), which are not visible from the flow alone, so no D2 conversion can + // rewrite the source mechanically. + { + id: 'flow-cel-record-variables-alias-retired', + // No backticks in `surface` — build-upgrade-guide renders it inside a code + // span already, and a nested backtick would close it. + surface: + 'flows[].nodes[].config.condition, flows[].edges[].condition, a decision node\'s conditions[].expression, a ' + + 'screen node\'s fields[].visibleWhen, and the CEL value envelopes of an assignment node\'s assignments and of ' + + 'a create_record or update_record node\'s fields — a CEL source reading record.X, or record bare, on a run ' + + 'that holds no record, where X names a flow variable', + replacement: + 'the variable by its name, or through the vars namespace: record.assignee becomes assignee, or vars.assignee. ' + + 'A record.X read on a run that was handed a record is unchanged, and reads that record\'s field X', + reason: + 'Flow CEL binds record to the record the run was handed: a record-change trigger\'s row, a time-relative ' + + 'sweep\'s row, the inbound hook\'s request body, a flow action\'s record (the loaded row, or an empty record ' + + 'carrying at most the id it was given), a subflow or map parent\'s record, or a map item that carries an id. ' + + 'A flow can also bind a variable named record itself. A run with none of these, one started through the REST ' + + 'trigger route, a declared endpoint or a cron schedule, or a child of such a run, used to see record bound to ' + + 'its own variables map, so record.X answered a flow variable named X instead of failing: a second spelling of ' + + 'every variable, which hid the author\'s mistake. Such a run now fails the expression with an unknown-variable ' + + 'fault on record, carrying the source, as every other unbound root does. No D2 conversion exists: whether a ' + + 'run holds a record depends on the entrances that start the flow, which the flow alone does not show, so ' + + 'only the author can tell a variable read from a record field read. Measured at the change: 0 flows in this ' + + 'repository\'s example apps, platform objects and dogfood suites, and 0 in objectstack-ai/hotcrm, read record ' + + 'with no record entrance.', + acceptanceCriteria: + 'For each flow whose CEL reads record, list the entrances that start it. Where any of them hands no record, ' + + 'replace each record.X that names a flow variable with the variable\'s name or vars.X. Re-run each flow path ' + + 'started with no record and confirm every expression evaluates instead of failing with an unknown-variable ' + + 'fault on record; re-run a path started with a record and confirm record.X still reads that record\'s field.', + }, // A flow CEL expression is refused when it reads a root the flow does not bind. // Formulas, row-level security and the client bind the run's user as user, // ctx.user and os.user as well as current_user; flow CEL binds current_user only,