diff --git a/.changeset/22565-flow-cel-unbound-root-refused.md b/.changeset/22565-flow-cel-unbound-root-refused.md index 077dee9a07f..cbddb31a642 100644 --- a/.changeset/22565-flow-cel-unbound-root-refused.md +++ b/.changeset/22565-flow-cel-unbound-root-refused.md @@ -11,11 +11,12 @@ Clause-②: no (narrowing) **BREAKING**: an accept-set narrowing on a published authoring surface (`objectstack validate`'s `expression-invalid` rule). -**Why.** Flow CEL evaluates in one scope per run: the flow's variables spread to top level, the trigger record's fields flattened beside them, and `record`, `previous`, `vars` and `current_user` bound by the engine. Formulas, row-level security and the client also bind the run's user as `user`, `ctx.user` and `os.user`; flow CEL does not, because a top-level `user` would collide with a variable or a lookup field of that name. So an author who wrote the alias by habit shipped a flow that faulted at its first evaluation, and nothing warned. +**Why.** Flow CEL evaluates in one scope per run: the flow's variables spread to top level, the trigger record's fields flattened beside them, `record` bound to the record the run was handed (where an entrance hands it one), and `previous`, `vars` and `current_user` bound by the engine. Formulas, row-level security and the client also bind the run's user as `user`, `ctx.user` and `os.user`; flow CEL does not, because a top-level `user` would collide with a variable or a lookup field of that name. So an author who wrote the alias by habit shipped a flow that faulted at its first evaluation, and nothing warned. **What is refused.** At every flow CEL site the build door reads — a node's `config.condition` (the start node's trigger gate included), an edge `condition`, a `decision` branch `expression`, a screen field `visibleWhen`, and the CEL value envelopes of an `assignment` and of a `create_record` / `update_record` `fields` map — a root that is none of: -- the engine's roots: `record`, `previous`, `vars`, `current_user`; +- the engine's roots: `previous`, `vars`, `current_user`; +- `record`, where an entrance the stack declares hands the flow a record — a record trigger (a `record-*` `triggerType`), a time-relative sweep, the inbound hook, a `type: 'flow'` action, a `map` node with an `itemObject`, or a `subflow` / `map` parent handed one — or where the flow binds a variable named `record`; - a name the flow binds: a declared variable, an `outputVariable`, an `iteratorVariable` (a `loop`'s default `item` included), an `indexVariable`, an `errorVariable`, an `assignment` target, a node id, a screen field's `name`; - a field (or a registry-injected column) of any object the stack declares — the run flattens whichever record its entrance hands it (an object-less flow action hands only the row `id`). @@ -45,7 +46,7 @@ A `$` name has no CEL spelling (`$runId == "x"` does not parse), so the `$` root **The one-line fix: write `current_user` for the run user, and bind or correct every other root the refusal names.** -**Who is affected, measured.** The last published spec line, 17.x, judged a flow expression's syntax only, so it accepts every refused source. Measured with the judge over every flow CEL site: the example apps (`app-crm`, `app-todo`, `app-multi-package`, `app-showcase`: 35 flows, 45 sites), `packages/platform-objects` (no flows) and `objectstack-ai/hotcrm` at `f0afcbda07` (32 flows, 65 sites) — 0 refusals; every root those flows read is an engine root, a binding of the flow's own or a field of its trigger object. One flow of the 67 is left unjudged by an entrance (`showcase_inbound_task_webhook`, an `api` trigger). Deployed metadata and other repositories were not measured. +**Who is affected, measured.** The last published spec line, 17.x, judged a flow expression's syntax only, so it accepts every refused source. Measured with the judge over every flow CEL site: the example apps (`app-crm`, `app-todo`, `app-multi-package`, `app-showcase`: 35 flows, 45 sites), `packages/platform-objects` (no flows) and `objectstack-ai/hotcrm` at `f0afcbda07` (32 flows, 65 sites) — 0 refusals; every root those flows read is an engine root, `record` on a flow an entrance hands a record, a binding of the flow's own or a field of its trigger object. One flow of the 67 is left unjudged by an entrance (`showcase_inbound_task_webhook`, an `api` trigger). Deployed metadata and other repositories were not measured. ### The kit diff --git a/.changeset/22642-flow-cel-record-binding.md b/.changeset/22642-flow-cel-record-binding.md index 4cadfa39a9d..715ec5ec02e 100644 --- a/.changeset/22642-flow-cel-record-binding.md +++ b/.changeset/22642-flow-cel-record-binding.md @@ -35,4 +35,4 @@ In a flow whose run holds no record, read a variable by its name: - 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. +Deployed metadata and other repositories were not measured. `objectstack validate` refuses such a `record` read too, naming the root (see `@objectstack/lint`'s note). diff --git a/.changeset/22677-flow-cel-record-entrance.md b/.changeset/22677-flow-cel-record-entrance.md new file mode 100644 index 00000000000..4d8e6b70785 --- /dev/null +++ b/.changeset/22677-flow-cel-record-entrance.md @@ -0,0 +1,31 @@ +--- +'@objectstack/lint': major +--- + +A flow CEL expression that reads `record` on a flow no entrance hands a record is refused at `objectstack validate`. Since #22642 the run binds `record` only to the record it was handed, so such a read fails the run with `Unknown variable: record`, and the build door still counted `record` as bound on every run and passed it. + +Clause-②: no (narrowing) + + + +**BREAKING**: an accept-set narrowing on a published authoring surface (`objectstack validate`'s `expression-invalid` rule). + +**What is refused.** `record` is no longer one of the roots the build door binds on every run (`previous`, `vars` and `current_user` still are). It is bound where: + +- an entrance the stack declares hands the flow a record: a record trigger (start `config.triggerType` is a `record-*` token), a time-relative sweep (start `config.timeRelative`), the inbound hook (trigger kind `api`), a `type: 'flow'` action that targets the flow (on any object, or on none), a `map` node with a `config.itemObject` that names the flow, or a `subflow` / `map` parent that is itself handed a record; +- or the flow binds a variable named `record` itself (a declared variable or an `assignment` target). + +A start `config.objectName` alone is no entrance: with no `record-*` trigger nothing hands the run a row. A parent's own `record` variable is not handed on either: a child gets its parent's context, not its variables. Everywhere else `record.X` is refused as `expression-invalid`, `error`, with the remedy below. + +**Not judged.** The stand-downs of the unbound-root rule are unchanged. At the runtime publish gate this judgment still stands down: its per-write snapshot carries no actions and no other flows, so it cannot see an entrance, and a flow write there is not refused for `record` until that snapshot carries them. + +## FROM → TO + +| you wrote | write instead | +|:--|:--| +| `record.assignee` on a flow no entrance hands a record, meaning a flow variable `assignee` | `assignee`, or `vars.assignee` | +| `record.status == "open"` on a flow meant to run when a row is written | the trigger that hands it the row: start `config: { objectName: 'acct', triggerType: 'record-after-update' }` | + +**The one-line fix: read a flow variable by its name or through `vars`, and read `record` only on a flow an entrance hands one.** + +**Who is affected, measured.** With this judge over every flow CEL site: the example apps (`app-crm`, `app-todo`, `app-multi-package`, `app-showcase`: 35 flows, 14 that read `record`), `packages/platform-objects` (no flows) and `objectstack-ai/hotcrm` at `1d7148b` (32 flows, 14 that read `record`): 0 newly refused. Every flow that reads `record` there is a record-change, time-relative or inbound-hook flow. Deployed metadata and other repositories were not measured. diff --git a/packages/lint/src/flow-cel-root-scope.ts b/packages/lint/src/flow-cel-root-scope.ts index 7e68e337be1..1f19fd39e07 100644 --- a/packages/lint/src/flow-cel-root-scope.ts +++ b/packages/lint/src/flow-cel-root-scope.ts @@ -26,13 +26,20 @@ * * ## The bound set — the flow's own static bindings * - * - **the engine's roots**: `record` and `previous` (`seedRunVariables` binds - * `previous` on every run, `null` on a create; `celScope` binds `record` to - * the trigger record, or to the variables themselves when there is none), - * `vars` and `current_user` (`celScope` binds both after the spread). A - * `$`-named variable has no CEL spelling at all — `$runId == "x"` does not - * parse — so the `$` names never reach this judge: they stay the text-slot - * rule's (`flow-text-slot-template.ts`), and this module keeps no list of them; + * - **the engine's roots**: `previous` (`seedRunVariables` binds it on every + * run, `null` when the run was handed none), `vars` and `current_user` + * (`celScope` binds both after the spread). A `$`-named variable has no CEL + * spelling at all — `$runId == "x"` does not parse — so the `$` names never + * reach this judge: they stay the text-slot rule's + * (`flow-text-slot-template.ts`), and this module keeps no list of them; + * - **`record` — the record the run was handed (#22677)**. `celScope` binds + * no `record` of its own (#22642): `seedRunVariables` binds `context.record` + * as `record` when the run's entrance hands it one, and nothing else does + * but a flow's own `record` variable (the next bullet). So `record` is bound + * here only when {@link flowCelEntrances} reads an entrance that hands this + * flow a record (the rule for records below), or the flow binds the name + * itself. An autolaunched flow with no visible entrance gets none, and its + * `record.X` is refused: the run faults `Unknown variable: record`; * - **every name the flow binds**: {@link collectFlowVariableNames} — declared * variables, `outputVariable` / `errorVariable` / `iteratorVariable` / * `indexVariable`, `assignment` targets, node ids — plus the two bindings that @@ -99,6 +106,30 @@ * carries `params` and `recordId` but no `record`, and the engine never loads a * record from `object` + `recordId`. A schedule run hands none either. * + * ## Two questions per entrance: does it open the flow, does it hand a record + * + * "Opens" (above) asks whether an entrance can hand a record whose keys are not + * in hand. "Hands a record" asks whether it hands one at all, and answers whether + * `record` is bound (#22677). A flow can be handed a record and still judged — + * its object is declared, so `record` is bound and its fields resolve. The + * entrances that hand a record, measured on the engine: + * + * - **a record trigger** — trigger kind `record_change` + * (`resolveFlowTriggerKind`: start `config.triggerType` is a `record-*` + * string), the row; a start `config.objectName` alone hands none, because with + * no `record-*` trigger nothing binds the flow to a write (`deriveTriggerBinding`); + * - **a time-relative sweep** — trigger kind `time_relative`, each swept row; + * - **the inbound hook** — trigger kind `api`, the request body; + * - **an action** — every `type: 'flow'` action that targets it, whatever its + * object: `dispatchFlowAction` hands the row it loaded, or an empty record + * carrying at most the `id` it was given, and an empty record is a record; + * - **a `map` node** with a `config.itemObject` — each id-bearing item becomes + * the child's record; + * - **a parent that is itself handed one** — a `subflow` or `map` node in a flow + * an entrance hands a record: the child spreads its parent's context, and so + * its `context.record`. A parent's own `record` VARIABLE is not handed on — + * the child gets the parent's context, not its variables. Read to a fixpoint. + * * ## The runtime publish gate stands down (#22636 is the carrier) * * The same pass runs at the runtime publish gate on a flow write, and there the @@ -132,11 +163,13 @@ type AnyRec = Record; /** * The roots the flow engine binds on every run, whatever the flow declares: - * `record` and `previous` (`seedRunVariables`, and `celScope`'s `record` arm), - * `vars` and `current_user` (bound by `celScope` after the variables are spread). - * Not the `$` names — those have no CEL spelling (module note). + * `previous` (`seedRunVariables`, `null` when the run was handed none), `vars` + * and `current_user` (bound by `celScope` after the variables are spread). + * Not `record`: it is the record the run was handed, bound only where an + * entrance hands one or the flow binds the name (module note, #22677). Not the + * `$` names — those have no CEL spelling (module note). */ -const ENGINE_BOUND_ROOTS: readonly string[] = ['record', 'previous', 'vars', 'current_user']; +const ENGINE_BOUND_ROOTS: readonly string[] = ['previous', 'vars', 'current_user']; /** * The node types whose variable writes this reader models completely — every @@ -219,7 +252,10 @@ export interface UnboundFlowCelRoot { export interface FlowCelEntrance { /** Set ⇒ the flow is not judged; names the entrance. */ readonly openedBy?: string; - /** Names an entrance binds beyond the objects' fields (an object-less action's `id`). */ + /** + * Names an entrance binds beyond the objects' fields: `record` when an + * entrance hands the flow a record, and an object-less action's `id`. + */ readonly bound?: readonly string[]; } @@ -301,13 +337,15 @@ function nameOf(value: unknown): string | undefined { /** * Every flow of the stack whose entrances open it, or bind a name beyond the * objects' fields — read from the stack's own declarations (the module's rule - * for records). A flow absent from the map is judged as its own text reads. + * for records). A flow an entrance hands a record gets `record` in + * {@link FlowCelEntrance.bound}. A flow absent from the map is judged as its own + * text reads, with no `record` unless it binds one itself. * * Built once per stack, because an entrance lives OUTSIDE the flow it feeds (an - * action, another flow's `subflow` / `map` node) and a parent's openness passes - * to its children: the parent edges are read to a fixpoint. A child named by a - * node but not declared in this stack is another package's flow and is not - * judged here. + * action, another flow's `subflow` / `map` node) and a parent's openness, and + * the record it was handed, pass to its children: the parent edges are read to + * a fixpoint. A child named by a node but not declared in this stack is another + * package's flow and is not judged here. * * @param fieldIndex object name → field names; an object is "declared" when it * is a key here, the same universe the bound set reads. @@ -320,15 +358,22 @@ export function flowCelEntrances( const flowNames = new Set(flows.map((flow) => nameOf(flow.name)).filter((name): name is string => !!name)); const openedBy = new Map(); const bound = new Map(); + // The flows an entrance hands a record — any object's — so `record` is bound. + const handed = new Set(); const open = (flow: string, reason: string): void => { if (flowNames.has(flow) && !openedBy.has(flow)) openedBy.set(flow, reason); }; + const hand = (flow: string): void => { + if (flowNames.has(flow)) handed.add(flow); + }; const undeclared = (object: string): boolean => !fieldIndex.has(object); // The flow's own trigger: a record trigger, a time-relative sweep, the inbound hook. for (const flow of flows) { const name = nameOf(flow.name); if (!name) continue; + const kind = resolveFlowTriggerKind(flow); + if (kind === 'record_change' || kind === 'time_relative' || kind === 'api') hand(name); const start = recordsOf(flow.nodes).find((node) => node.type === 'start'); const config = start ? configOf(start) : {}; const triggerObject = nameOf(config.objectName); @@ -340,7 +385,7 @@ export function flowCelEntrances( if (sweepObject && undeclared(sweepObject)) { open(name, `its time-relative sweep's object '${sweepObject}' (start config.timeRelative.object) is not declared in this stack, so the record's keys are not in hand`); } - if (resolveFlowTriggerKind(flow) === 'api') { + if (kind === 'api') { open(name, 'its trigger kind is api, so the inbound hook hands the request body in as the record, whatever its keys'); } } @@ -356,6 +401,7 @@ export function flowCelEntrances( for (const { action, object } of launches) { const target = action.type === 'flow' ? nameOf(action.target) : undefined; if (!target || !flowNames.has(target)) continue; + hand(target); if (object === undefined) { // Object-less: the action hands an empty record, carrying at most the row id it was given. bound.set(target, [...(bound.get(target) ?? []), 'id']); @@ -384,6 +430,7 @@ export function flowCelEntrances( edges.push({ parent, child, kind: node.type, node: nodeId }); if (node.type === 'map') { const itemObject = nameOf(config.itemObject); + if (itemObject) hand(child); if (!itemObject) { open(child, `map node '${nodeId}' in flow '${parent}' feeds it items and declares no config.itemObject, so the items' object is not in hand`); } else if (undeclared(itemObject)) { @@ -396,6 +443,10 @@ export function flowCelEntrances( for (let changed = true; changed;) { changed = false; for (const edge of edges) { + if (handed.has(edge.parent) && !handed.has(edge.child)) { + handed.add(edge.child); + changed = true; + } if (!openedBy.has(edge.parent) || openedBy.has(edge.child)) continue; open(edge.child, `${edge.kind} node '${edge.node}' in flow '${edge.parent}' hands it that flow's record, and '${edge.parent}' is itself open`); changed = true; @@ -405,9 +456,9 @@ export function flowCelEntrances( const out = new Map(); for (const name of flowNames) { const reason = openedBy.get(name); - const names = bound.get(name); - if (reason === undefined && names === undefined) continue; - out.set(name, { ...(reason !== undefined ? { openedBy: reason } : {}), ...(names !== undefined ? { bound: names } : {}) }); + const names = [...(handed.has(name) ? ['record'] : []), ...(bound.get(name) ?? [])]; + if (reason === undefined && names.length === 0) continue; + out.set(name, { ...(reason !== undefined ? { openedBy: reason } : {}), ...(names.length > 0 ? { bound: names } : {}) }); } return out; } @@ -526,8 +577,10 @@ export function unboundFlowCelRoots(source: string, scope: FlowCelRootScope): Un * The refusal for one unbound root. * * A run-user spelling (`user`, `ctx.user`, `os.user`) is told to write - * `current_user`; any other root is named with what the flow could have bound - * it, and the nearest in-scope name when there is one. + * `current_user`; `record` with no entrance that hands the flow one is told to + * read the variable by its name or through `vars` (the remedy #22642 names), or + * to give the flow its entrance; any other root is named with what the flow + * could have bound it, and the nearest in-scope name when there is one. */ export function unboundFlowCelRootMessage(unbound: UnboundFlowCelRoot, scope: FlowCelRootScope): string { const { root, members } = unbound; @@ -541,12 +594,24 @@ export function unboundFlowCelRootMessage(unbound: UnboundFlowCelRoot, scope: Fl 'without a user, guard it: `current_user != null ? current_user.id : null`.' ); } + if (root === 'record') { + const member = members[0] ?? 'assignee'; + return ( + '`record` is not bound in this flow\'s expression scope, so the run fails with `Unknown variable: record`: ' + + 'a flow\'s `record` is the record its run was handed, and no entrance in this stack hands this flow one — no ' + + 'record trigger (start `config.triggerType: \'record-…\'`), time-relative sweep, inbound hook, `type: \'flow\'` ' + + 'action, `map` node with a `config.itemObject`, or `subflow` / `map` parent that is handed a record — and the ' + + 'flow binds no variable named `record`. ' + + `Read a variable by its name (\`record.${member}\` → \`${member}\`) or through \`vars\` (\`vars.${member}\`), ` + + 'or give the flow the entrance that hands it its record.' + ); + } const near = nearestName(root, scope.bound); return ( `\`${root}\` is not bound in this flow's expression scope, so the run fails with \`Unknown variable: ${root}\`: ` + 'no variable, `outputVariable`, iterator, index or error variable, `assignment` target, node id or screen field ' + `of this flow is named \`${root}\`, no object in the stack declares a field \`${root}\`, and the engine binds ` + - 'only `record`, `previous`, `vars` and `current_user`. ' + + 'only `previous`, `vars` and `current_user` on every run, and `record` when an entrance hands the run one. ' + `Declare \`${root}\` as a flow variable or bind it with a node, or correct the name` + (near ? ` — did you mean \`${near}\`?` : '.') ); diff --git a/packages/lint/src/lint-flow-patterns.test.ts b/packages/lint/src/lint-flow-patterns.test.ts index b17f6c7ed06..083f9fca9bd 100644 --- a/packages/lint/src/lint-flow-patterns.test.ts +++ b/packages/lint/src/lint-flow-patterns.test.ts @@ -2779,12 +2779,16 @@ describe('#16405 — an `http` node payload is not a region, and both #1315 rule // read as the token it means, asked at the string's position; elsewhere the // single brace still resolves and the hint is unchanged. describe('a double brace in a value slot', () => { + // [#22677] The prescriptions read `record.title`, so the flow is handed a + // record (a record trigger on a declared `task`): with no entrance the + // build door refuses `record` itself, whatever the slot's spelling. function valueFlow(type: string, config: Record) { return { + objects: [{ name: 'task', fields: { title: { type: 'text' }, subject: { type: 'text' } } }], flows: [{ - name: 'stamp_title', label: 'Stamp title', type: 'autolaunched', + name: 'stamp_title', label: 'Stamp title', type: 'record_change', nodes: [ - { id: 'start', type: 'start', label: 'Start' }, + { id: 'start', type: 'start', label: 'Start', config: { objectName: 'task', triggerType: 'record-after-update' } }, { id: 'write', type, label: 'Write', config }, { id: 'done', type: 'end', label: 'Done' }, ], diff --git a/packages/lint/src/validate-expressions.flow-cel-root.test.ts b/packages/lint/src/validate-expressions.flow-cel-root.test.ts index e783dcd73be..794173fb048 100644 --- a/packages/lint/src/validate-expressions.flow-cel-root.test.ts +++ b/packages/lint/src/validate-expressions.flow-cel-root.test.ts @@ -13,7 +13,11 @@ * The judge lives in `flow-cel-root-scope.ts`; these pins drive it through * `validateStackExpressions`, the pass `objectstack validate` runs. The runtime * publish gate runs the same pass and this one judgment stands down there (the - * last block below). + * S block below). + * + * #22677: `record` is no longer bound on every run — it is the record the run + * was handed (#22642), bound where an entrance hands the flow one or the flow + * binds the name (the last block below). */ import { describe, expect, it } from 'vitest'; @@ -33,6 +37,8 @@ function flowStack(opts: { variables?: AnyRec[]; middle?: AnyRec[]; objectName?: string; + /** The start node's `config.triggerType` — a `record-*` token is the record trigger that hands the run a row. */ + triggerType?: string; objects?: AnyRec[]; }): AnyRec { return { @@ -48,6 +54,7 @@ function flowStack(opts: { type: 'start', config: { ...(opts.objectName ? { objectName: opts.objectName } : {}), + ...(opts.triggerType ? { triggerType: opts.triggerType } : {}), ...(opts.startCondition !== undefined ? { condition: opts.startCondition } : {}), }, }, @@ -172,11 +179,12 @@ describe('controls — one per binding kind the run has', () => { ['a trigger field, read bare', { edgeCondition: 'status == "open" && amount > 5', objectName: 'acct' }], ['a field of another object the stack declares (an action, a subflow parent or a map item can hand its record in)', { edgeCondition: 'region == "eu"', objectName: 'acct', objects: [...OBJECTS, { name: 'branch', fields: { region: { type: 'text' } } }] }], ['a registry-injected column', { edgeCondition: 'owner_id != null', objectName: 'acct' }], - ['the engine roots', { edgeCondition: 'record != null && previous == null && vars.x == null && current_user != null' }], + ['the engine roots', { edgeCondition: 'previous == null && vars.x == null && current_user != null' }], + ['`record`, on a flow a record trigger hands one (#22677)', { edgeCondition: 'record != null && record.status == "open"', objectName: 'acct', triggerType: 'record-after-update' }], ['a `$` variable through `vars` (a `$` name has no CEL spelling)', { edgeCondition: 'vars["$record"] != null' }], ['a comprehension macro variable', { edgeCondition: '[1, 2].exists(user, user > 1) && [1].map(x, x > 0, x + 1).size() == 1' }], ['a `cel.bind` variable', { edgeCondition: 'cel.bind(user, 2, user > 1)' }], - ['a CEL type name', { edgeCondition: 'type(record) == map && int("1") == 1' }], + ['a CEL type name', { edgeCondition: 'type(vars) == map && int("1") == 1' }], ])('%s', (_kind, opts) => { expect(rootFindings(flowStack(opts))).toEqual([]); }); @@ -233,8 +241,9 @@ describe('the wider family — any root nothing binds (rides the same judge)', ( expect(found[0]!.message).toBe( '`foo` is not bound in this flow\'s expression scope, so the run fails with `Unknown variable: foo`: no variable, ' + '`outputVariable`, iterator, index or error variable, `assignment` target, node id or screen field of this flow is ' - + 'named `foo`, no object in the stack declares a field `foo`, and the engine binds only `record`, `previous`, ' - + '`vars` and `current_user`. Declare `foo` as a flow variable or bind it with a node, or correct the name.', + + 'named `foo`, no object in the stack declares a field `foo`, and the engine binds only `previous`, `vars` and ' + + '`current_user` on every run, and `record` when an entrance hands the run one. Declare `foo` as a flow variable ' + + 'or bind it with a node, or correct the name.', ); }); @@ -400,3 +409,109 @@ describe('the runtime publish gate (S)', () => { expect(scope.openedBy).toContain('per-write snapshot'); }); }); + +/** + * #22677 — `record` is the record the run was handed (#22642): `seedRunVariables` + * binds `context.record` and `celScope` binds no `record` of its own. So the + * build door binds it only where an entrance the stack declares hands the flow a + * record, or the flow binds a variable named `record`; otherwise `record.X` is + * refused. The runtime publish gate's stand-down (S) covers it until #22636. + */ +describe('`record` — bound only where an entrance hands the flow a record (#22677)', () => { + const READ = 'record.status == "open"'; + const RECORD_TRIGGER = { objectName: 'acct', triggerType: 'record-after-update' }; + const RECORD_HEAD = /^`record` is not bound in this flow's expression scope, so the run fails with `Unknown variable: record`/; + /** `f`'s refusals of `record` — another flow's findings and other roots are not this pin's. */ + const recordRefusals = (stack: AnyRec) => + rootFindings(stack).filter((i) => i.where.startsWith("flow 'f'") && RECORD_HEAD.test(i.message)); + /** A second flow `g` whose `node` names `f`; `start` is its start config, `variables` its declarations. */ + const parent = (opts: { name?: string; start?: AnyRec; type?: string; variables?: AnyRec[]; node: AnyRec }): AnyRec => ({ + name: opts.name ?? 'g', + label: 'G', + type: opts.type ?? 'autolaunched', + ...(opts.variables ? { variables: opts.variables } : {}), + nodes: [ + { id: 'start', type: 'start', config: opts.start ?? {} }, + opts.node, + { id: 'done', type: 'end', config: {} }, + ], + edges: [{ id: 'p1', source: 'start', target: opts.node.id }, { id: 'p2', source: opts.node.id, target: 'done' }], + }); + const calls = (child: string): AnyRec => ({ id: 'call', type: 'subflow', config: { flowName: child } }); + const withFlows = (stack: AnyRec, ...flows: AnyRec[]): AnyRec => ({ ...stack, flows: [...(stack.flows as AnyRec[]), ...flows] }); + + it('`validate` refuses `record.X` on a flow no entrance hands a record, with the `vars` remedy', () => { + const found = errorsOf(flowStack({ edgeCondition: READ })); + expect(found).toHaveLength(1); + expect(found[0]!.where).toBe("flow 'f' · edge 'e1' (start→done) condition"); + expect(found[0]!.message).toMatch(RECORD_HEAD); + expect(found[0]!.message).toContain('(`record.status` → `status`) or through `vars` (`vars.status`)'); + }); + + it('a start config.objectName alone is no entrance — with no `record-*` trigger nothing hands the run a row', () => { + expect(recordRefusals(flowStack({ edgeCondition: READ, objectName: 'acct' }))).toHaveLength(1); + }); + + it('control: a record-triggered flow passes', () => { + expect(recordRefusals(flowStack({ edgeCondition: READ, ...RECORD_TRIGGER }))).toEqual([]); + expect(validateStackExpressions(flowStack({ edgeCondition: READ, ...RECORD_TRIGGER }))).toEqual([]); + }); + + it('control: a `subflow` child of a record-handing parent passes, to a fixpoint', () => { + expect(recordRefusals(withFlows(flowStack({ edgeCondition: READ }), parent({ start: RECORD_TRIGGER, node: calls('f') })))).toEqual([]); + // Two levels: g (record trigger) → h → f. + expect(recordRefusals(withFlows( + flowStack({ edgeCondition: READ }), + parent({ start: RECORD_TRIGGER, node: calls('h') }), + parent({ name: 'h', node: calls('f') }), + ))).toEqual([]); + }); + + it('RED CONTROL — a `subflow` child of a parent no entrance hands a record is refused', () => { + expect(recordRefusals(withFlows(flowStack({ edgeCondition: READ }), parent({ node: calls('f') })))).toHaveLength(1); + }); + + it('a parent\'s own `record` VARIABLE is not handed on — the child gets its context, not its variables', () => { + const declares = parent({ variables: [{ name: 'record', type: 'object' }], node: calls('f') }); + expect(recordRefusals(withFlows(flowStack({ edgeCondition: READ }), declares))).toHaveLength(1); + }); + + it.each<[string, (stack: AnyRec) => AnyRec]>([ + ['a time-relative sweep', (stack) => { + ((((stack.flows as AnyRec[])[0]!.nodes as AnyRec[])[0]) as AnyRec).config = { timeRelative: { object: 'acct', dateField: 'due', withinDays: 7 } }; + return stack; + }], + ['an action on a declared object', (stack) => ({ ...stack, actions: [{ name: 'go', label: 'Go', type: 'flow', target: 'f', objectName: 'acct' }] })], + ['an object-less action (it hands an empty record, still a record)', (stack) => ({ ...stack, actions: [{ name: 'go', label: 'Go', type: 'flow', target: 'f' }] })], + ['a `map` node with a declared itemObject', (stack) => withFlows(stack, parent({ node: { id: 'each', type: 'map', config: { collection: '{rows}', flowName: 'f', itemObject: 'acct' } } }))], + ['a declared `record` variable', (stack) => ({ ...stack, flows: [{ ...(stack.flows as AnyRec[])[0]!, variables: [{ name: 'record', type: 'object' }] }] })], + ])('control: %s', (_entrance, give) => { + expect(recordRefusals(give(flowStack({ edgeCondition: READ })))).toEqual([]); + }); + + it('the reader binds `record` per entrance, and the inbound hook\'s too (that flow also stands down)', () => { + const fieldIndex = new Map([['acct', ['status']]]); + const flow = (name: string, start: AnyRec, type = 'autolaunched'): AnyRec => ({ name, type, nodes: [{ id: 'start', type: 'start', config: start }] }); + const entrances = flowCelEntrances({ + flows: [flow('trig', RECORD_TRIGGER), flow('hook', {}, 'api'), flow('bare', { objectName: 'acct' }), flow('act', {})], + actions: [{ name: 'go', type: 'flow', target: 'act' }], + }, fieldIndex); + expect(entrances.get('trig')).toEqual({ bound: ['record'] }); + expect(entrances.get('hook')?.bound).toEqual(['record']); + expect(entrances.get('act')).toEqual({ bound: ['record', 'id'] }); + expect(entrances.has('bare')).toBe(false); + }); + + it('the runtime publish gate does not refuse `record.X` on a flow write, while `validate` refuses it on the same flow', () => { + const written = { + name: 'f', + label: 'F', + type: 'autolaunched', + nodes: [{ id: 'start', type: 'start', config: {} }, { id: 'done', type: 'end', config: {} }], + edges: [{ id: 'e1', source: 'start', target: 'done', condition: READ }], + }; + const result = runRuntimeAuthoringRules({ type: 'flow', item: written, context: { objects: OBJECTS } }); + expect(result.errors.filter((f) => / is not bound in /.test(f.message))).toEqual([]); + expect(recordRefusals({ objects: OBJECTS, flows: [written] })).toHaveLength(1); + }); +}); diff --git a/packages/lint/src/validate-expressions.test.ts b/packages/lint/src/validate-expressions.test.ts index 812605ae001..4104e5c149b 100644 --- a/packages/lint/src/validate-expressions.test.ts +++ b/packages/lint/src/validate-expressions.test.ts @@ -44,7 +44,7 @@ describe('validateStackExpressions (ADR-0032 build-time)', () => { flows: [{ name: 'lead_flow', nodes: [ - { id: 'start', type: 'start', config: { objectName: 'crm_lead' } }, + { id: 'start', type: 'start', config: { objectName: 'crm_lead', triggerType: 'record-after-update' } }, { id: 'check', type: 'decision', config: { condition: 'record.rating >= 4' } }, ], edges: [{ id: 'e1', source: 'check', target: 'end', condition: 'record.rating < 4' }], @@ -78,7 +78,7 @@ describe('validateStackExpressions (ADR-0032 build-time)', () => { flows: [{ name: 'lead_flow', nodes: [ - { id: 'start', type: 'start', config: { objectName: 'crm_lead' } }, + { id: 'start', type: 'start', config: { objectName: 'crm_lead', triggerType: 'record-after-update' } }, { id: 'check', type: 'decision', config: { condition: 'record.raitng >= 4' } }, ], edges: [], @@ -475,7 +475,7 @@ describe('validateStackExpressions (ADR-0032 build-time)', () => { name: 'record_change', variables: [{ name: 'status', type: 'text' }], nodes: [ - { id: 'start', type: 'start', config: { objectName: 'duly_assignment', condition: 'record.status == "dispatched"' } }, + { id: 'start', type: 'start', config: { objectName: 'duly_assignment', triggerType: 'record-after-update', condition: 'record.status == "dispatched"' } }, ], edges: [], }], @@ -2757,7 +2757,7 @@ describe('validateStackExpressions (ADR-0032 build-time)', () => { objects, flows: [{ name: 'sweep', - nodes: [{ id: 'start', type: 'start', config: { objectName: 'crm_lead' } }, container], + nodes: [{ id: 'start', type: 'start', config: { objectName: 'crm_lead', triggerType: 'record-after-update' } }, container], edges: [], }], }); @@ -3108,8 +3108,8 @@ describe('null-guard gate (#4763)', () => { // #4811 re-measured the reason this one is excluded. It is NOT the // flattened scope (this gate never resolves a bare identifier — only - // `record.`/`previous.`, and the engine binds both roots - // unconditionally). It is that `record-change-trigger.ts` seeds the flow's + // `record.`/`previous.`, and a record-triggered run binds both + // roots — #22677). It is that `record-change-trigger.ts` seeds the flow's // record as `{ ...(inputData ?? {}), ...after }` (spelled `inputDoc` until // #5671 dropped that alias read) with no `materializeDeclaredFields`, // so a declared column the write never mentioned is an ABSENT key — and on @@ -3122,7 +3122,7 @@ describe('null-guard gate (#4763)', () => { flows: [{ name: 'escalate', nodes: [ - { id: 'start', type: 'start', config: { objectName: 'showcase_project' } }, + { id: 'start', type: 'start', config: { objectName: 'showcase_project', triggerType: 'record-after-update' } }, { id: 'd', type: 'decision', config: { condition: 'record.budget > 100000' } }, ], edges: [{ id: 'e1', source: 'd', target: 'end', condition: 'record.spent > record.budget' }], @@ -4028,7 +4028,7 @@ describe('validateStackExpressions — injected system columns (#5378)', () => { flows: [{ name: 'contact_welcome', nodes: [ - { id: 'start', type: 'start', config: { objectName: 'crm_contact', condition } }, + { id: 'start', type: 'start', config: { objectName: 'crm_contact', triggerType: 'record-after-update', condition } }, ], edges: [], }], @@ -4453,7 +4453,7 @@ describe('structural condition shape (#15662)', () => { { id: 'start', type: 'start', config: { - objectName: 'crm_lead', + objectName: 'crm_lead', triggerType: 'record-after-update', ...('startCondition' in opts ? { condition: opts.startCondition } : {}), }, }, @@ -4620,7 +4620,7 @@ describe('blank structural condition (#17495)', () => { { id: 'start', type: 'start', config: { - objectName: 'crm_lead', + objectName: 'crm_lead', triggerType: 'record-after-update', ...('startCondition' in opts ? { condition: opts.startCondition } : {}), }, }, @@ -4751,7 +4751,7 @@ describe("validateStackExpressions — a non-record entry in an object's `fields flows: [{ name: 'account_flow', nodes: [ - { id: 'start', type: 'start', config: { objectName: 'crm_account' } }, + { id: 'start', type: 'start', config: { objectName: 'crm_account', triggerType: 'record-after-update' } }, { id: 'check', type: 'decision', config: { condition } }, ], edges: [], diff --git a/packages/spec/src/migrations/entries/semantic/18.flow-cel-unbound-root-refused.ts b/packages/spec/src/migrations/entries/semantic/18.flow-cel-unbound-root-refused.ts index eecfbcc5705..696d5b2849e 100644 --- a/packages/spec/src/migrations/entries/semantic/18.flow-cel-unbound-root-refused.ts +++ b/packages/spec/src/migrations/entries/semantic/18.flow-cel-unbound-root-refused.ts @@ -25,7 +25,9 @@ export const entry: SemanticMigration = { + 'record.FIELD', reason: 'A flow CEL expression evaluates in one scope per run: the flow\'s variables spread to top level, the trigger ' - + 'record\'s fields flattened beside them, and record, previous, vars and current_user bound by the engine. A ' + + 'record\'s fields flattened beside them, record bound to the record the run was handed (where an entrance the ' + + 'stack declares hands the flow one, or the flow binds a variable named record), and previous, vars and ' + + 'current_user bound by the engine. A ' + 'root outside that scope fails the expression with an unknown-variable fault on every run that reaches it, ' + 'and the build door, which judged a flow expression\'s syntax only, passed it. The run-user spellings are ' + 'the reachable case: formulas, row-level security and the client bind the run\'s user under user, ctx.user ' diff --git a/packages/spec/src/migrations/registry.ts b/packages/spec/src/migrations/registry.ts index 9aaae83bdec..87c176a0d81 100644 --- a/packages/spec/src/migrations/registry.ts +++ b/packages/spec/src/migrations/registry.ts @@ -13829,7 +13829,9 @@ const step18: MigrationStep = { + 'record.FIELD', reason: 'A flow CEL expression evaluates in one scope per run: the flow\'s variables spread to top level, the trigger ' - + 'record\'s fields flattened beside them, and record, previous, vars and current_user bound by the engine. A ' + + 'record\'s fields flattened beside them, record bound to the record the run was handed (where an entrance the ' + + 'stack declares hands the flow one, or the flow binds a variable named record), and previous, vars and ' + + 'current_user bound by the engine. A ' + 'root outside that scope fails the expression with an unknown-variable fault on every run that reaches it, ' + 'and the build door, which judged a flow expression\'s syntax only, passed it. The run-user spellings are ' + 'the reachable case: formulas, row-level security and the client bind the run\'s user under user, ctx.user '