diff --git a/.changeset/22565-flow-cel-unbound-root-refused.md b/.changeset/22565-flow-cel-unbound-root-refused.md new file mode 100644 index 00000000000..077dee9a07f --- /dev/null +++ b/.changeset/22565-flow-cel-unbound-root-refused.md @@ -0,0 +1,53 @@ +--- +'@objectstack/lint': major +'@objectstack/spec': major +--- + +A flow CEL expression that reads a root the flow does not bind is refused at `objectstack validate`. The run fails such an expression with `Unknown variable: X` (X being the root) on every evaluation, and the build door used to pass it. The reachable case is the run user: `user.id == "u1"`, `ctx.user.id == "u1"` and `os.user.id == "u1"` — the spellings formulas, row-level security and the client accept — are refused, naming `current_user`, the one spelling flow CEL binds. + +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. + +**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`; +- 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`). + +The user spellings are told to write `current_user`, with the guard for a flow that can run without a user; any other root is named with what could have bound it, and the nearest in-scope name. The error is `expression-invalid`, `error`. Where the flattened-scope near-miss advisory ("`X` is not a field of … If `X` is a flow variable this is safe to ignore") named the same root, the refusal replaces it. + +**Not judged — the door stands down where the run can bind a name it cannot see.** + +- **A node:** a `script` or `connector_action` node (each hands its code the live variable map), a `wait`, `subflow` or `map` node (a resume folds the caller's `variables` bag in under their plain names), a screen with no field list, or a node type outside the builtin set. +- **An entrance the stack declares that hands the flow a record whose keys are not in hand:** + - a record trigger (`config.objectName`) or a time-relative sweep (`config.timeRelative.object`) on an object the stack does not declare; + - an `api` trigger, whose inbound hook hands the request body in as the record; + - a `type: 'flow'` action launching the flow on an undeclared object; + - a `map` node feeding the flow whose `itemObject` is undeclared or absent; + - a `subflow` or `map` parent that is itself such a flow. +- **The runtime publish gate:** a flow write there is not judged for unbound roots yet. Its per-write snapshot carries no actions or other flows, so it cannot see those entrances. Every other expression verdict at that door is unchanged. + +A `$` name has no CEL spelling (`$runId == "x"` does not parse), so the `$` roots stay the text-slot rule's. + +## FROM → TO + +| you wrote | write instead | +|:--|:--| +| `condition: 'user.id == "u1"'` | `condition: 'current_user.id == "u1"'` (`current_user != null && current_user.id == "u1"` where the flow can run without a user) | +| `condition: 'ctx.user.id == record.owner'` | `condition: 'current_user.id == record.owner'` | +| `assignments: { who: { dialect: 'cel', source: 'os.user.id' } }` | `assignments: { who: { dialect: 'cel', source: 'current_user.id' } }` | +| `condition: 'deals.size() > 0'` with nothing binding `deals` | declare it (`variables: [{ name: 'deals', type: 'list' }]`) or bind it with a node (`outputVariable: 'deals'`) | + +**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. + +### The kit + +- **The judge.** `flow-cel-root-scope.ts` in `@objectstack/lint`, called by `validateStackExpressions` at each flow CEL site. One bound set per flow, read once like the shadowing pass's variable set, and one entrance index per stack (`flowCelEntrances`). +- **The ledger.** The D3 semantic entry `flow-cel-unbound-root-refused` (protocol 18). There is no D2 conversion: none of these roots ever evaluated in a flow, so there is no behaviour to carry over. diff --git a/packages/lint/src/flow-cel-root-scope.ts b/packages/lint/src/flow-cel-root-scope.ts new file mode 100644 index 00000000000..7e68e337be1 --- /dev/null +++ b/packages/lint/src/flow-cel-root-scope.ts @@ -0,0 +1,553 @@ +// Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license. + +/** + * @module flow-cel-root-scope + * + * **The build door's judge of the ROOTS a flow CEL expression reads.** A flow + * expression whose root identifier the run does not bind fails that run with + * `Unknown variable: X` — measured on the flow scope the engine builds + * (`AutomationEngine.celScope` over `seedRunVariables`' map): `user.id == "u1"`, + * `ctx.user.id == "u1"`, `os.user.id == "u1"` and `foo.bar == 1` each fault, while + * `objectstack validate` answered nothing for any of them. The three user + * spellings are the trap: formulas, row-level security and the client bind the + * run's user under them (`buildScope` in `@objectstack/formula`), and flow CEL + * binds it as `current_user` only, so an author who writes the alias by habit + * ships a flow that faults. This module refuses such a root at build, naming + * `current_user` for the user spellings and the in-scope names for any other. + * + * ## One judge, every flow CEL site + * + * The caller (`validateStackExpressions`) hands it each flow CEL source it + * already visits: a node's `config.condition` (the start node's is the trigger + * gate), an edge's `condition`, a `decision` branch's `expression`, a `screen` + * field's `visibleWhen`, and the CEL value envelopes of an `assignment` and of a + * `create_record` / `update_record` `fields` map. All of them evaluate in the + * one scope `celScope` builds, so one bound set per flow judges them all. + * + * ## 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; + * - **every name the flow binds**: {@link collectFlowVariableNames} — declared + * variables, `outputVariable` / `errorVariable` / `iteratorVariable` / + * `indexVariable`, `assignment` targets, node ids — plus the two bindings that + * reader does not collect: a `screen` field's `name` (a resume lands the + * collected values under their plain names) and a `loop`'s default iterator + * `item` when the node names none; + * - **record fields**: the trigger record's fields are flattened to top-level + * names. Every field of EVERY object the stack declares counts, with the + * registry-injected columns — not only the trigger object's — because the run + * flattens whichever record its entrance hands it (the entrances below). A + * field the stack declares anywhere is therefore a root this door cannot + * prove unbound. An object-less action that launches the flow hands an empty + * record carrying at most the row `id` it was given, so `id` is bound there. + * + * ## When the door cannot prove a root unbound, it stands down + * + * A false refusal of a working flow is worse than the trap, so the whole flow is + * left unjudged when its run can bind a name the reader cannot see: + * + * - a node whose type is not one of {@link CLOSED_BINDER_NODE_TYPES}. The node + * type namespace is open (ADR-0018), and the executors that hand out the LIVE + * variable map are open by measurement: `script` (a registered function gets + * `FlowFunctionContext.variables`) and `connector_action` + * (`ConnectorActionContext.variables`) may write any name; `wait`, `subflow` + * and `map` declare `resumeAuthority: 'any'`, and a resume folds the caller's + * `variables` bag in under its plain names (`applyResumeSignal`); + * - a `screen` with no input contract — an `object-form` screen, or one that + * declares no fields — because `refuseInvalidScreenInput` checks a resume bag + * only against a declared field list, so a contract-less screen folds any bag; + * - an ENTRANCE that hands the flow a record whose keys are not in hand + * ({@link flowCelEntrances}, the rule below). + * + * ## The rule for records: every entrance the stack declares is read + * + * The run flattens whichever record its entrance hands it (`seedRunVariables` + * reads `context.record` and nothing else), so the record half of the bound set + * holds only while every entrance that can hand this flow a record names an + * object the stack declares. {@link flowCelEntrances} reads each entrance from + * the stack's own declarations and opens the flow when one does not: + * + * - **a record trigger** — the start node's `config.objectName` (the trigger + * object a record-change run's row belongs to) names an undeclared object; + * - **a time-relative sweep** — the start node's `config.timeRelative.object` + * (`TimeRelativeTriggerSchema`; its own key, independent of `objectName`, and + * the object whose rows the sweep hands each run) is undeclared; + * - **the inbound hook** — the flow's trigger kind is `api` + * (`resolveFlowTriggerKind`: `type: 'api'` or `config.triggerType: 'api'`), so + * the hook hands the request body in as the record (`trigger-api`), keys and + * all; + * - **an action** — an `ActionSchema` entry with `type: 'flow'` and + * `target: ` in `actions[]` (its `objectName`) or in + * `objects[].actions[]` (its `objectName`, else the owning object) names an + * undeclared object: `dispatchFlowAction` hands the action's own record to + * whatever flow it targets; + * - **a `map` node** — `MapConfigSchema`'s `flowName` names this flow and its + * `itemObject` is undeclared or absent: each id-bearing item becomes the + * child's record, and with no `itemObject` the items' object is not in hand; + * - **a parent that is itself open** — a `subflow` node (`SubflowConfigSchema`'s + * `flowName`) or a `map` node names this flow inside a flow that is open: the + * child spreads its parent's context, record included. Read to a fixpoint. + * + * The API run door hands no record (measured): both REST trigger routes and the + * declarative endpoint build their context with `buildAutomationContext`, which + * carries `params` and `recordId` but no `record`, and the engine never loads a + * record from `object` + `recordId`. A schedule run hands none either. + * + * ## 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 + * per-write snapshot carries the written flow alone beside `objects` / + * `permissions` / `books` / `datasets` (`buildRuntimeWriteSnapshotSet`): no + * action and no other flow, so no entrance is visible and a parent's `subflow` or + * `map` node could hand this flow any record. A false 422 on the only door a + * Studio tenant has is worse than the gap, so this one judgment stands down there + * ({@link perWriteSnapshotEntrance}); every other expression verdict at that door + * is unchanged, and `objectstack validate` judges under the rule above. #22636 + * widens that snapshot to carry flows and actions; when it lands, this + * stand-down goes. + */ + +import { firstUndeclaredReference, parseCelToAst, SCOPE_ROOTS } from '@objectstack/formula'; +import { + APPROVAL_NODE_TYPE, + APPROVAL_REVISE_NODE_TYPE, + collectFlowGraphs, + LOOP_NODE_TYPE, + PARALLEL_NODE_TYPE, + resolveFlowTriggerKind, + TRY_CATCH_NODE_TYPE, +} from '@objectstack/spec/automation'; +import type { FlowEdgeParsed, FlowNodeParsed } from '@objectstack/spec/automation'; + +import { collectFlowVariableNames, type FlowGraphLike, type FlowVariableHost } from './flow-variable-scope.js'; +import { nearestName, recordsOf } from './object-graph.js'; + +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). + */ +const ENGINE_BOUND_ROOTS: readonly string[] = ['record', 'previous', 'vars', 'current_user']; + +/** + * The node types whose variable writes this reader models completely — every + * name each one can bind is a key of its own `config` that + * {@link collectFlowVariableNames} or {@link flowCelRootScope} reads, or its node + * id. Measured from the executors: + * + * | type | binds | + * |---|---| + * | `start`, `end`, `decision`, `http`, `notify`, `delete_record` | nothing but its node id's outputs | + * | `assignment` | its targets | + * | `create_record`, `update_record`, `get_record` | `outputVariable` | + * | `loop` | `iteratorVariable` (default `item`), `indexVariable` | + * | `parallel` | nothing; its branches are graphs of their own | + * | `try_catch` | `errorVariable` (default `$error`) | + * | `screen` with an input contract | its fields' names, `outputVariable` | + * | `approval`, `approval_revise` | nothing; their service resumes with outputs only (`resumeAuthority: 'service'`) | + * + * Every other type — a plugin's, and `script`, `connector_action`, `wait`, + * `subflow`, `map` (module note) — makes the bound set unprovable. + */ +const CLOSED_BINDER_NODE_TYPES: ReadonlySet = new Set([ + 'start', + 'end', + 'decision', + 'assignment', + 'create_record', + 'update_record', + 'delete_record', + 'get_record', + 'http', + 'notify', + 'screen', + LOOP_NODE_TYPE, + PARALLEL_NODE_TYPE, + TRY_CATCH_NODE_TYPE, + APPROVAL_NODE_TYPE, + APPROVAL_REVISE_NODE_TYPE, +]); + +/** The run-user spellings formulas, RLS and the client accept and flow CEL does not bind. */ +const RUN_USER_ALIAS_ROOTS: ReadonlySet = new Set(['user', 'ctx', 'os']); + +/** What the door knows about one flow's CEL scope. */ +export interface FlowCelRootScope { + /** + * Every root the run can bind, in the order a did-you-mean prefers them: the + * trigger object's fields first (a flattened condition's commonest typo), then + * the flow's own bindings, the engine's roots, and the other objects' fields. + * Meaningful only when {@link provable}. + */ + readonly bound: ReadonlySet; + /** + * `false` ⇒ the run can bind a name the reader cannot see, and no root of this + * flow is judged. {@link openedBy} says why. + */ + readonly provable: boolean; + /** The first reason the bound set is not provable, for tests and diagnostics. */ + readonly openedBy?: string; +} + +/** The flow-level slice the scope reads. */ +export interface FlowCelRootHost extends FlowVariableHost { + readonly nodes?: unknown; +} + +/** One root a flow CEL source reads that the flow does not bind. */ +export interface UnboundFlowCelRoot { + /** The root identifier (`user`). */ + readonly root: string; + /** The member names read directly off it (`user.id` → `id`), in source order. */ + readonly members: readonly string[]; +} + +/** + * What the flow's entrances contribute to its scope ({@link flowCelEntrances}): + * the reason one of them hands it a record whose keys are not in hand, and any + * name an entrance binds that no object field covers. + */ +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`). */ + readonly bound?: readonly string[]; +} + +/** + * The bound set of one flow, or the reason it cannot be proved. + * + * @param graphs every graph of the flow (`collectFlowGraphs`), so a region's + * nodes count — one variable map serves the whole run. + * @param triggerObject the start node's `objectName`, when it names one — its + * fields lead the did-you-mean order. + * @param fieldIndex object name → its field names, injected columns included. + * @param entrance what the flow's entrances contribute: from + * {@link flowCelEntrances} at the build door, {@link perWriteSnapshotEntrance} + * at the runtime publish gate. Its reason takes precedence over a node's. + */ +export function flowCelRootScope( + flow: FlowCelRootHost, + graphs: readonly FlowGraphLike[], + triggerObject: string | undefined, + fieldIndex: ReadonlyMap, + entrance: FlowCelEntrance = {}, +): FlowCelRootScope { + const bound = new Set(triggerObject !== undefined ? fieldIndex.get(triggerObject) ?? [] : []); + for (const name of collectFlowVariableNames(flow, graphs)) bound.add(name); + for (const name of ENGINE_BOUND_ROOTS) bound.add(name); + for (const name of entrance.bound ?? []) bound.add(name); + let openedBy: string | undefined = entrance.openedBy; + const open = (reason: string): void => { openedBy ??= reason; }; + + for (const graph of graphs) { + for (const node of recordsOf(graph.nodes)) { + const type = typeof node.type === 'string' ? node.type : ''; + const id = typeof node.id === 'string' ? node.id : '?'; + if (!CLOSED_BINDER_NODE_TYPES.has(type)) { + open(`node '${id}' (${type || 'no type'}) can bind names the reader cannot see`); + continue; + } + const config = (node.config && typeof node.config === 'object' && !Array.isArray(node.config) + ? node.config + : {}) as AnyRec; + if (type === LOOP_NODE_TYPE && !(typeof config.iteratorVariable === 'string' && config.iteratorVariable)) { + // `LoopConfigSchema` defaults the iterator to `item`; a config read before + // that parse carries no key, and the run still binds the default. + bound.add('item'); + } + if (type === 'screen') { + const fields = Array.isArray(config.fields) ? recordsOf(config.fields) : []; + if (config.kind === 'object-form' || fields.length === 0) { + open(`screen '${id}' declares no input contract, so a resume can bind any name`); + continue; + } + for (const field of fields) { + if (typeof field.name === 'string' && field.name) bound.add(field.name); + } + } + } + } + + for (const names of fieldIndex.values()) for (const name of names) bound.add(name); + + return openedBy === undefined ? { bound, provable: true } : { bound, provable: false, openedBy }; +} + +/** The stack slice {@link flowCelEntrances} reads. */ +export interface FlowCelEntranceHost { + readonly flows?: unknown; + readonly actions?: unknown; + readonly objects?: unknown; +} + +function configOf(node: AnyRec): AnyRec { + return node.config && typeof node.config === 'object' && !Array.isArray(node.config) ? (node.config as AnyRec) : {}; +} + +function nameOf(value: unknown): string | undefined { + return typeof value === 'string' && value ? value : 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. + * + * 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. + * + * @param fieldIndex object name → field names; an object is "declared" when it + * is a key here, the same universe the bound set reads. + */ +export function flowCelEntrances( + stack: FlowCelEntranceHost, + fieldIndex: ReadonlyMap, +): ReadonlyMap { + const flows = recordsOf(stack.flows); + const flowNames = new Set(flows.map((flow) => nameOf(flow.name)).filter((name): name is string => !!name)); + const openedBy = new Map(); + const bound = new Map(); + const open = (flow: string, reason: string): void => { + if (flowNames.has(flow) && !openedBy.has(flow)) openedBy.set(flow, reason); + }; + 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 start = recordsOf(flow.nodes).find((node) => node.type === 'start'); + const config = start ? configOf(start) : {}; + const triggerObject = nameOf(config.objectName); + if (triggerObject && undeclared(triggerObject)) { + open(name, `its trigger object '${triggerObject}' (start config.objectName) is not declared in this stack, so the record's keys are not in hand`); + } + const sweep = config.timeRelative && typeof config.timeRelative === 'object' ? (config.timeRelative as AnyRec) : undefined; + const sweepObject = sweep ? nameOf(sweep.object) : undefined; + 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') { + open(name, 'its trigger kind is api, so the inbound hook hands the request body in as the record, whatever its keys'); + } + } + + // Actions that launch a flow: `actions[]`, and each object's own `actions[]`. + const launches: Array<{ action: AnyRec; object: string | undefined }> = []; + for (const action of recordsOf(stack.actions)) launches.push({ action, object: nameOf(action.objectName) }); + for (const object of recordsOf(stack.objects)) { + for (const action of recordsOf(object.actions)) { + launches.push({ action, object: nameOf(action.objectName) ?? nameOf(object.name) }); + } + } + for (const { action, object } of launches) { + const target = action.type === 'flow' ? nameOf(action.target) : undefined; + if (!target || !flowNames.has(target)) continue; + 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']); + } else if (undeclared(object)) { + open(target, `action '${nameOf(action.name) ?? '?'}' launches it on '${object}' (the action's objectName), which this stack does not declare`); + } + } + + // Parent edges: a `subflow` or `map` node naming a flow of this stack. + const edges: Array<{ parent: string; child: string; kind: 'subflow' | 'map'; node: string }> = []; + for (const flow of flows) { + const parent = nameOf(flow.name); + if (!parent) continue; + const graphs = collectFlowGraphs({ + ...flow, + nodes: recordsOf(flow.nodes) as unknown as FlowNodeParsed[], + edges: recordsOf(flow.edges) as unknown as FlowEdgeParsed[], + }); + for (const graph of graphs) { + for (const node of recordsOf(graph.nodes)) { + if (node.type !== 'subflow' && node.type !== 'map') continue; + const config = configOf(node); + const child = nameOf(config.flowName); + if (!child || !flowNames.has(child)) continue; + const nodeId = nameOf(node.id) ?? '?'; + edges.push({ parent, child, kind: node.type, node: nodeId }); + if (node.type === 'map') { + const itemObject = nameOf(config.itemObject); + 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)) { + open(child, `map node '${nodeId}' in flow '${parent}' feeds it items of '${itemObject}' (config.itemObject), which this stack does not declare`); + } + } + } + } + } + for (let changed = true; changed;) { + changed = false; + for (const edge of edges) { + 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; + } + } + + 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 } : {}) }); + } + return out; +} + +/** + * The runtime publish gate's entrance: none visible. Its per-write snapshot + * carries the written flow alone (module note), so this one judgment stands down + * there until #22636 widens the snapshot. + */ +export function perWriteSnapshotEntrance(): FlowCelEntrance { + return { + openedBy: + 'the runtime publish gate judges a flow write against a per-write snapshot that carries no action and no ' + + 'other flow, so no entrance that can hand this flow a record is visible', + }; +} + +/** cel-js's comprehension macros: the receiver calls that bind their first argument in the rest. */ +const COMPREHENSION_MACROS: ReadonlySet = new Set(['all', 'exists', 'exists_one', 'map', 'filter']); + +interface CelNode { + readonly op: string; + readonly args: unknown; +} + +function isCelNode(value: unknown): value is CelNode { + return !!value && typeof value === 'object' && !Array.isArray(value) && typeof (value as AnyRec).op === 'string'; +} + +function boundIdentifier(value: unknown): string | undefined { + return isCelNode(value) && value.op === 'id' && typeof value.args === 'string' ? value.args : undefined; +} + +/** + * The FREE root identifiers of a parsed CEL source, with the members read off + * each. A name a comprehension macro (`all`, `exists`, `exists_one`, `map`, + * `filter`) or `cel.bind` binds is free only outside the part it is bound in — + * the same scoping cel-js applies (`lib/macros.js`). + */ +function freeRoots(ast: unknown): Map { + const out = new Map(); + const note = (name: string, member: string | undefined): void => { + const members = out.get(name) ?? []; + if (member !== undefined && !members.includes(member)) members.push(member); + out.set(name, members); + }; + const walk = (node: unknown, scoped: ReadonlySet, member?: string): void => { + if (Array.isArray(node)) { + for (const child of node) walk(child, scoped); + return; + } + if (!isCelNode(node)) return; + const args = node.args; + if (node.op === 'id') { + if (typeof args === 'string' && !scoped.has(args)) note(args, member); + return; + } + if ((node.op === '.' || node.op === '.?') && Array.isArray(args)) { + walk(args[0], scoped, typeof args[1] === 'string' ? args[1] : undefined); + return; + } + if (node.op === 'rcall' && Array.isArray(args)) { + const [name, receiver, list] = args as [unknown, unknown, unknown]; + const variable = Array.isArray(list) ? boundIdentifier(list[0]) : undefined; + if (typeof name === 'string' && COMPREHENSION_MACROS.has(name) && Array.isArray(list) && list.length >= 2 && variable) { + walk(receiver, scoped); + walk(list.slice(1), new Set([...scoped, variable])); + return; + } + if (name === 'bind' && boundIdentifier(receiver) === 'cel' && Array.isArray(list) && list.length === 3 && variable) { + walk(list[1], scoped); + walk(list[2], new Set([...scoped, variable])); + return; + } + walk(receiver, scoped); + walk(list, scoped); + return; + } + if (node.op === 'value') return; + walk(args, scoped); + }; + walk(ast, new Set()); + return out; +} + +/** + * Whether `name`, read as a bare identifier, is a VARIABLE the run must bind — + * as opposed to a name CEL declares itself (a type such as `int`, the `cel` + * namespace). Answered by the strict checker, which knows every such name; a + * platform scope root (`user`, `ctx`, `os`, `data`, …) is declared there as a + * convenience for other surfaces, and is a variable here. + */ +function isVariableReference(name: string): boolean { + if ((SCOPE_ROOTS as readonly string[]).includes(name)) return true; + return firstUndeclaredReference(name) === name; +} + +/** + * The roots `source` reads that `scope` does not bind, in source order — empty + * when the scope is not provable, or when the source does not parse (the syntax + * check owns that verdict: one broken expression, one finding). + */ +export function unboundFlowCelRoots(source: string, scope: FlowCelRootScope): UnboundFlowCelRoot[] { + if (!scope.provable) return []; + const ast = parseCelToAst(source); + if (!ast) return []; + const out: UnboundFlowCelRoot[] = []; + for (const [root, members] of freeRoots(ast)) { + if (scope.bound.has(root) || !isVariableReference(root)) continue; + out.push({ root, members }); + } + return out; +} + +/** + * 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. + */ +export function unboundFlowCelRootMessage(unbound: UnboundFlowCelRoot, scope: FlowCelRootScope): string { + const { root, members } = unbound; + const userSpelling = root === 'user' ? 'user' : RUN_USER_ALIAS_ROOTS.has(root) && members.includes('user') ? `${root}.user` : undefined; + if (userSpelling !== undefined) { + return ( + `\`${userSpelling}\` is not bound in a flow expression, so the run fails with \`Unknown variable: ${root}\`: ` + + 'a flow binds the run\'s user as `current_user` only — the `user`, `ctx.user` and `os.user` spellings that ' + + 'formulas, row-level security and the client accept are not bound here. ' + + `Write \`current_user\` in its place (\`${userSpelling}.id\` → \`current_user.id\`); for a flow that can run ` + + 'without a user, guard it: `current_user != null ? current_user.id : null`.' + ); + } + 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`. ' + + `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 9352e9a1e2b..9d357842dbe 100644 --- a/packages/lint/src/lint-flow-patterns.test.ts +++ b/packages/lint/src/lint-flow-patterns.test.ts @@ -2612,6 +2612,11 @@ describe('#16405 — an `http` node payload is not a region, and both #1315 rule return { flows: [{ name: 'stamp_owner', label: 'Stamp owner', type: 'autolaunched', + // The variables the prescriptions below read, bound as their hints + // say they must be: the flow's own `source` (`$source`'s remedy) and + // the `me` a `get_record` binds (`$User.Email`'s). The build door + // refuses a root the flow does not bind (#22565), as the run does. + variables: [{ name: 'source', type: 'object' }, { name: 'me', type: 'object' }], nodes: [ { id: 'start', type: 'start', label: 'Start' }, { id: 'write', type, label: 'Write', config }, diff --git a/packages/lint/src/validate-expressions.flow-cel-root.test.ts b/packages/lint/src/validate-expressions.flow-cel-root.test.ts new file mode 100644 index 00000000000..e783dcd73be --- /dev/null +++ b/packages/lint/src/validate-expressions.flow-cel-root.test.ts @@ -0,0 +1,402 @@ +// Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license. + +/** + * #22565 — the build door refuses a flow CEL root the flow does not bind. + * + * Measured on the flow scope the engine builds (`AutomationEngine.celScope` + * over `seedRunVariables`' map): `user.id == "u1"`, `ctx.user.id == "u1"`, + * `os.user.id == "u1"` and `foo.bar == 1` each fail the run with + * `Unknown variable: `, while `validateStackExpressions` answered 0 issues + * for every one of them. The three user spellings are the ones formulas, RLS and + * the client bind; flow CEL binds the run's user as `current_user` only. + * + * 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). + */ + +import { describe, expect, it } from 'vitest'; + +import { flowCelEntrances, flowCelRootScope, perWriteSnapshotEntrance, unboundFlowCelRoots } from './flow-cel-root-scope.js'; +import { runRuntimeAuthoringRules } from './runtime-gate.js'; +import { validateStackExpressions } from './validate-expressions.js'; + +type AnyRec = Record; + +const OBJECTS = [{ name: 'acct', fields: { status: { type: 'text' }, amount: { type: 'number' } } }]; + +/** start → (edge carrying `edgeCondition`) → done, plus any `middle` nodes, on an optional trigger object. */ +function flowStack(opts: { + edgeCondition?: unknown; + startCondition?: unknown; + variables?: AnyRec[]; + middle?: AnyRec[]; + objectName?: string; + objects?: AnyRec[]; +}): AnyRec { + return { + objects: opts.objects ?? OBJECTS, + flows: [{ + name: 'f', + label: 'F', + type: 'autolaunched', + ...(opts.variables ? { variables: opts.variables } : {}), + nodes: [ + { + id: 'start', + type: 'start', + config: { + ...(opts.objectName ? { objectName: opts.objectName } : {}), + ...(opts.startCondition !== undefined ? { condition: opts.startCondition } : {}), + }, + }, + ...(opts.middle ?? []), + { id: 'done', type: 'end', config: {} }, + ], + edges: [ + { id: 'e1', source: 'start', target: 'done', ...(opts.edgeCondition !== undefined ? { condition: opts.edgeCondition } : {}) }, + ], + }], + }; +} + +const errorsOf = (stack: AnyRec) => validateStackExpressions(stack).filter((i) => (i.severity ?? 'error') === 'error'); +/** + * The judge's own findings — every refusal it writes says the root "is not bound + * in" a flow scope. Controls and stand-downs read these, so a node-config refusal + * about an unrelated key can neither mask nor fake a verdict here. + */ +const rootFindings = (stack: AnyRec) => errorsOf(stack).filter((i) => / is not bound in (a flow expression|this flow's expression scope)/.test(i.message)); +const CURRENT_USER_REMEDY = 'a flow binds the run\'s user as `current_user` only'; + +describe('the triage pins (#22565)', () => { + it('an edge condition `user.id == "u1"` is refused with the `current_user` remedy', () => { + const found = errorsOf(flowStack({ edgeCondition: 'user.id == "u1"' })); + expect(found).toHaveLength(1); + expect(found[0]!.where).toBe("flow 'f' · edge 'e1' (start→done) condition"); + expect(found[0]!.source).toBe('user.id == "u1"'); + expect(found[0]!.message).toMatch(/^`user` is not bound in a flow expression, so the run fails with `Unknown variable: user`/); + expect(found[0]!.message).toContain(CURRENT_USER_REMEDY); + expect(found[0]!.message).toContain('(`user.id` → `current_user.id`)'); + }); + + it('control: the same condition on a flow that declares a variable `user` passes', () => { + expect(validateStackExpressions(flowStack({ + edgeCondition: 'user.id == "u1"', + variables: [{ name: 'user', type: 'object' }], + }))).toEqual([]); + }); + + it('control: the remedy itself — `current_user.id == "u1"` — passes', () => { + expect(validateStackExpressions(flowStack({ edgeCondition: 'current_user.id == "u1"' }))).toEqual([]); + }); +}); + +describe('every run-user spelling formulas, RLS and the client accept', () => { + it.each([ + ['ctx.user.id == "u1"', 'ctx.user', 'ctx'], + ['os.user.id == "u1"', 'os.user', 'os'], + ['has(user.id)', 'user', 'user'], + ['user.?id.orValue("") == "u1"', 'user', 'user'], + ['"admin" in user.positions', 'user', 'user'], + ])('`%s` is refused naming `current_user`', (source, spelling, root) => { + const found = errorsOf(flowStack({ edgeCondition: source })); + expect(found).toHaveLength(1); + expect(found[0]!.message.startsWith(`\`${spelling}\` is not bound in a flow expression, so the run fails with \`Unknown variable: ${root}\``)).toBe(true); + expect(found[0]!.message).toContain(CURRENT_USER_REMEDY); + expect(found[0]!.message).toContain(`(\`${spelling}.id\` → \`current_user.id\`)`); + }); + + it('`ctx` / `os` read for something other than the user get the general refusal, naming the root', () => { + const found = errorsOf(flowStack({ edgeCondition: 'os.env == "prod"' })); + expect(found).toHaveLength(1); + expect(found[0]!.message).toMatch(/^`os` is not bound in this flow's expression scope/); + expect(found[0]!.message).not.toContain('current_user` in its place'); + }); +}); + +describe('every flow CEL site the door visits', () => { + const USER = 'user.id == "u1"'; + const env = (source: string) => ({ dialect: 'cel', source }); + + it.each([ + ['the start node\'s trigger gate', flowStack({ startCondition: USER }), "flow 'f' · node 'start' (start) condition"], + ['an edge', flowStack({ edgeCondition: USER }), "flow 'f' · edge 'e1' (start→done) condition"], + ['another node\'s `config.condition`', flowStack({ middle: [{ id: 'gate', type: 'decision', config: { condition: USER } }] }), + "flow 'f' · node 'gate' (decision) condition"], + ['a decision branch', flowStack({ middle: [{ id: 'pick', type: 'decision', config: { conditions: [{ label: 'y', expression: USER }] } }] }), + "flow 'f' · node 'pick' (decision) decision branch expression at config.conditions[0].expression"], + ['a screen field `visibleWhen`', flowStack({ middle: [{ id: 'ask', type: 'screen', config: { fields: [{ name: 'note', type: 'text', visibleWhen: USER }] } }] }), + "flow 'f' · node 'ask' (screen) screen field visibleWhen at config.fields[0].visibleWhen"], + ['an assignment value envelope', flowStack({ middle: [{ id: 'set', type: 'assignment', config: { assignments: { who: env('user.id') } } }] }), + "flow 'f' · node 'set' (assignment) assignment value at config.assignments.who"], + ['a create_record field envelope', flowStack({ middle: [{ id: 'mk', type: 'create_record', config: { objectName: 'acct', fields: { status: env('user.id') } } }] }), + "flow 'f' · node 'mk' (create_record) create_record field value at config.fields.status"], + ['an update_record field envelope', flowStack({ middle: [{ id: 'up', type: 'update_record', config: { objectName: 'acct', filter: { id: '{record.id}' }, fields: { status: env('user.id') } } }] }), + "flow 'f' · node 'up' (update_record) update_record field value at config.fields.status"], + ])('%s', (_site, stack, where) => { + const found = rootFindings(stack); + expect(found.map((i) => i.where)).toEqual([where]); + expect(found[0]!.message).toContain(CURRENT_USER_REMEDY); + }); + + it('an expression inside an ADR-0031 region body is judged against the same flow-wide set', () => { + const found = rootFindings(flowStack({ + middle: [{ + id: 'sweep', type: 'loop', + config: { collection: '{rows}', iteratorVariable: 'row', body: { nodes: [{ id: 'inner', type: 'decision', config: { conditions: [{ label: 'y', expression: 'user.id == row.owner' }] } }], edges: [] } }, + }], + variables: [{ name: 'rows', type: 'list' }], + })); + expect(found).toHaveLength(1); + expect(found[0]!.where).toContain("loop 'sweep' body"); + expect(found[0]!.message).toContain(CURRENT_USER_REMEDY); + }); + + it('a value slot\'s plain string is a literal, so its text is not read for roots', () => { + expect(rootFindings(flowStack({ middle: [{ id: 'set', type: 'assignment', config: { assignments: { who: 'user.id' } } }] }))).toEqual([]); + }); +}); + +describe('controls — one per binding kind the run has', () => { + it.each<[string, Parameters[0]]>([ + ['a declared variable', { edgeCondition: 'quota > 1', variables: [{ name: 'quota', type: 'number' }] }], + ['an `outputVariable`', { edgeCondition: 'acc.status == "x"', middle: [{ id: 'read', type: 'get_record', config: { objectName: 'acct', filter: { id: '{record.id}' }, outputVariable: 'acc' } }] }], + ['a node id (its outputs land under it)', { edgeCondition: 'read.record != null', middle: [{ id: 'read', type: 'get_record', config: { objectName: 'acct', filter: { id: '{record.id}' } } }] }], + ['an `assignment` target', { edgeCondition: 'flag == true', middle: [{ id: 'set', type: 'assignment', config: { assignments: { flag: true } } }] }], + ['a `loop` iterator and index', { edgeCondition: 'row != null && i >= 0', variables: [{ name: 'rows', type: 'list' }], middle: [{ id: 'sweep', type: 'loop', config: { collection: '{rows}', iteratorVariable: 'row', indexVariable: 'i', body: { nodes: [], edges: [] } } }] }], + ['a `loop` with no iterator binds the default `item`', { edgeCondition: 'item != null', variables: [{ name: 'rows', type: 'list' }], middle: [{ id: 'sweep', type: 'loop', config: { collection: '{rows}', body: { nodes: [], edges: [] } } }] }], + ['a `try_catch` `errorVariable`', { edgeCondition: 'caught != null', middle: [{ id: 'guard', type: 'try_catch', config: { errorVariable: 'caught', try: { nodes: [], edges: [] }, catch: { nodes: [], edges: [] } } }] }], + ['a screen field (a resume binds it under its name)', { edgeCondition: 'approve == true', middle: [{ id: 'ask', type: 'screen', config: { fields: [{ name: 'approve', type: 'boolean' }] } }] }], + ['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' }], + ['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' }], + ])('%s', (_kind, opts) => { + expect(rootFindings(flowStack(opts))).toEqual([]); + }); + + it('a binding declared by a node AFTER the reading edge still counts — one variable map serves the whole run', () => { + expect(rootFindings(flowStack({ + edgeCondition: 'later == 1', + middle: [{ id: 'tail', type: 'assignment', config: { assignments: { later: 1 } } }], + }))).toEqual([]); + }); + + it('a `cel.bind` variable is bound in its body only — the initializer reads the flow\'s scope', () => { + const found = rootFindings(flowStack({ edgeCondition: 'cel.bind(v, v, v > 1)' })); + expect(found).toHaveLength(1); + expect(found[0]!.message).toMatch(/^`v` is not bound in this flow's expression scope/); + }); +}); + +describe('the door stands down where the run can bind a name it cannot see', () => { + it.each<[string, AnyRec]>([ + ['a `script` (its function gets the live variable map)', { id: 'fn', type: 'script', config: { function: 'enrich' } }], + ['a `connector_action` (its handler gets the live variable map)', { id: 'push', type: 'connector_action', config: { connectorId: 'slack', actionId: 'post' } }], + ['a `wait` (a resume folds the caller\'s bag)', { id: 'hold', type: 'wait', config: { eventType: 'signal', signalName: 'go' } }], + ['a `subflow` (a resume folds the caller\'s bag)', { id: 'child', type: 'subflow', config: { flowName: 'other' } }], + ['a `map` (a resume folds the caller\'s bag)', { id: 'each', type: 'map', config: { collection: '{rows}', flowName: 'other' } }], + ['a plugin node type', { id: 'ai', type: 'ai_classify', config: {} }], + ['a screen with no fields', { id: 'ok', type: 'screen', config: { waitForInput: true } }], + ['an object-form screen', { id: 'form', type: 'screen', config: { kind: 'object-form', objectName: 'acct', idVariable: 'saved' } }], + ])('%s', (_why, node) => { + expect(rootFindings(flowStack({ edgeCondition: 'user.id == "u1" && foo == 1', middle: [node] }))).toEqual([]); + }); + + it('a trigger object the stack does not declare', () => { + expect(rootFindings(flowStack({ edgeCondition: 'user.id == "u1"', objectName: 'sys_user' }))).toEqual([]); + }); + + it('RED CONTROL — the same condition on the same flow with no opening node is refused, twice', () => { + expect(rootFindings(flowStack({ edgeCondition: 'user.id == "u1" && foo == 1' })).map((i) => i.message.slice(0, 6))) + .toEqual(['`user`', '`foo` ']); + }); + + it('the scope says why', () => { + const scope = flowCelRootScope({ nodes: [] }, [{ nodes: [{ id: 'fn', type: 'script', config: {} }] }], undefined, new Map()); + expect(scope.provable).toBe(false); + expect(scope.openedBy).toContain("node 'fn' (script)"); + expect(unboundFlowCelRoots('user.id == "u1"', scope)).toEqual([]); + }); +}); + +describe('the wider family — any root nothing binds (rides the same judge)', () => { + it('`foo.bar == 1` is refused naming the root and what could have bound it', () => { + const found = errorsOf(flowStack({ edgeCondition: 'foo.bar == 1' })); + expect(found).toHaveLength(1); + 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.', + ); + }); + + it('each unbound root is its own finding; a bound one beside it is not', () => { + const found = errorsOf(flowStack({ edgeCondition: 'alpha == 1 && status == "x" && beta == 2', objectName: 'acct' })); + expect(found.map((i) => i.message.slice(0, 7))).toEqual(['`alpha`', '`beta` ']); + }); + + it('a near-miss of a trigger field is ONE finding: the refusal carries the did-you-mean, the advisory is withdrawn', () => { + const issues = validateStackExpressions(flowStack({ edgeCondition: 'stauts == "open"', objectName: 'acct' })); + expect(issues).toHaveLength(1); + expect(issues[0]!.severity).toBe('error'); + expect(issues[0]!.message).toContain('did you mean `status`?'); + }); + + it('a source that does not parse is the syntax check\'s alone — one broken expression, one finding', () => { + const issues = validateStackExpressions(flowStack({ edgeCondition: 'user.id ==' })); + expect(issues).toHaveLength(1); + expect(issues[0]!.message).not.toContain('is not bound'); + }); +}); + +/** + * A′ (triage `6094431558`): every entrance the stack declares is read, and a flow + * an entrance hands a record of an undeclared (or unknowable) object is not + * judged. One pin per entrance: the undeclared case stands down, and the same + * flow fed by a declared object is still judged. + */ +describe('the entrances the stack declares (A′)', () => { + const USER = 'user.id == "u1"'; + /** A second flow `g` that names `f` from a node of the given type. */ + const parentFlow = (opts: { type?: string; start?: AnyRec; node: AnyRec }): AnyRec => ({ + name: 'g', + label: 'G', + type: opts.type ?? 'autolaunched', + nodes: [ + { id: 'start', type: 'start', config: opts.start ?? {} }, + opts.node, + { id: 'done', type: 'end', config: {} }, + ], + edges: [{ id: 'g1', source: 'start', target: opts.node.id }, { id: 'g2', source: opts.node.id, target: 'done' }], + }); + const withFlows = (stack: AnyRec, ...flows: AnyRec[]): AnyRec => ({ ...stack, flows: [...(stack.flows as AnyRec[]), ...flows] }); + const userRefusals = (stack: AnyRec) => rootFindings(stack).filter((i) => i.where.startsWith("flow 'f'")); + + it('a record trigger: an undeclared start config.objectName stands down; a declared one is judged', () => { + expect(userRefusals(flowStack({ edgeCondition: USER, objectName: 'sys_user' }))).toEqual([]); + expect(userRefusals(flowStack({ edgeCondition: USER, objectName: 'acct' }))).toHaveLength(1); + }); + + it('a time-relative sweep: an undeclared config.timeRelative.object stands down; a declared one is judged', () => { + const sweep = (object: string): AnyRec => { + const stack = flowStack({ edgeCondition: USER }); + const flow = (stack.flows as AnyRec[])[0]!; + (flow.nodes as AnyRec[])[0] = { id: 'start', type: 'start', config: { timeRelative: { object, dateField: 'due', withinDays: 7 } } }; + return stack; + }; + expect(userRefusals(sweep('sys_task'))).toEqual([]); + expect(userRefusals(sweep('acct'))).toHaveLength(1); + }); + + it('the inbound hook: a flow whose trigger kind is api stands down; the same flow as autolaunched is judged', () => { + const asApi = flowStack({ edgeCondition: USER }); + (asApi.flows as AnyRec[])[0]!.type = 'api'; + expect(userRefusals(asApi)).toEqual([]); + const viaTriggerType = flowStack({ edgeCondition: USER }); + (((viaTriggerType.flows as AnyRec[])[0]!.nodes as AnyRec[])[0] as AnyRec).config = { triggerType: 'api' }; + expect(userRefusals(viaTriggerType)).toEqual([]); + expect(userRefusals(flowStack({ edgeCondition: USER }))).toHaveLength(1); + }); + + it('an action: one launching the flow on an undeclared object stands down; on a declared object it is judged', () => { + const launched = (objectName: string): AnyRec => ({ + ...flowStack({ edgeCondition: USER }), + actions: [{ name: 'go', label: 'Go', type: 'flow', target: 'f', objectName }], + }); + expect(userRefusals(launched('sys_user'))).toEqual([]); + expect(userRefusals(launched('acct'))).toHaveLength(1); + // An object's own actions[] carry the owning object. + const nested = flowStack({ + edgeCondition: USER, + objects: [{ ...OBJECTS[0], actions: [{ name: 'go', label: 'Go', type: 'flow', target: 'f' }] }], + }); + expect(userRefusals(nested)).toHaveLength(1); + // A non-flow action naming the flow's name is no entrance. + expect(userRefusals({ ...flowStack({ edgeCondition: USER }), actions: [{ name: 'go', label: 'Go', type: 'url', target: 'f', objectName: 'sys_user' }] })).toHaveLength(1); + }); + + it('an object-less action binds only the row id it is given', () => { + const bare = flowStack({ edgeCondition: 'id == "r1"', objects: [] }); + expect(rootFindings(bare).map((i) => i.message.slice(0, 4))).toEqual(['`id`']); + expect(rootFindings({ ...bare, actions: [{ name: 'go', label: 'Go', type: 'flow', target: 'f' }] })).toEqual([]); + }); + + it('a map node: an undeclared or absent itemObject stands down the flow it feeds; a declared one is judged', () => { + const fedBy = (itemObject?: string): AnyRec => withFlows( + flowStack({ edgeCondition: USER }), + parentFlow({ node: { id: 'each', type: 'map', config: { collection: '{rows}', flowName: 'f', ...(itemObject ? { itemObject } : {}) } } }), + ); + expect(userRefusals(fedBy('sys_task'))).toEqual([]); + expect(userRefusals(fedBy(undefined))).toEqual([]); + expect(userRefusals(fedBy('acct'))).toHaveLength(1); + }); + + it('a parent that is itself open: its subflow child stands down, to a fixpoint; a judged parent leaves the child judged', () => { + const subflowOf = (parent: AnyRec, child = 'f'): AnyRec => ({ ...parent, nodes: (parent.nodes as AnyRec[]).map((n) => (n.type === 'subflow' ? { ...n, config: { flowName: child } } : n)) }); + const call = { id: 'call', type: 'subflow', config: {} }; + const openParent = subflowOf(parentFlow({ type: 'api', node: call })); + expect(userRefusals(withFlows(flowStack({ edgeCondition: USER }), openParent))).toEqual([]); + // Two levels: g (the inbound hook) → h → f. + const middle = { ...subflowOf(parentFlow({ node: call })), name: 'h', label: 'H' }; + expect(userRefusals(withFlows(flowStack({ edgeCondition: USER }), subflowOf(openParent, 'h'), middle))).toEqual([]); + // A parent fed by a declared trigger object is judged, and so is its child. + const judgedParent = subflowOf(parentFlow({ start: { objectName: 'acct' }, node: call })); + expect(userRefusals(withFlows(flowStack({ edgeCondition: USER }), judgedParent))).toHaveLength(1); + }); + + it('the reader names the entrance', () => { + const entrances = flowCelEntrances( + { + flows: [{ name: 'f', type: 'autolaunched', nodes: [{ id: 'start', type: 'start' }] }], + actions: [{ name: 'go', type: 'flow', target: 'f', objectName: 'sys_user' }], + }, + new Map([['acct', ['status']]]), + ); + expect(entrances.get('f')?.openedBy).toContain("action 'go' launches it on 'sys_user'"); + }); +}); + +/** + * S (triage `6094850599`): at the runtime publish gate the per-write snapshot + * holds the written flow alone, so no entrance is visible and this one judgment + * stands down there. Every other expression verdict at that door is unchanged. + * #22636 widens the snapshot; when it lands, this stand-down goes. + */ +describe('the runtime publish gate (S)', () => { + 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: 'user.id == "u1"' }, + { id: 'e2', source: 'start', target: 'done', condition: '{record.status} == "open"' }, + ], + }; + + it('a flow write is not refused for an unbound root, while another expression verdict still refuses there', () => { + const result = runRuntimeAuthoringRules({ type: 'flow', item: written, context: { objects: OBJECTS } }); + const expressionErrors = result.errors.filter((f) => f.rule === 'expression-invalid'); + expect(expressionErrors.some((f) => / is not bound in /.test(f.message))).toBe(false); + expect(expressionErrors.some((f) => f.message.includes('template brace') && f.where.includes("edge 'e2'"))).toBe(true); + }); + + it('control: the build door refuses the same root on the same flow', () => { + const found = rootFindings({ objects: OBJECTS, flows: [written] }); + expect(found.map((i) => i.where)).toEqual(["flow 'f' · edge 'e1' (start→done) condition"]); + }); + + it('the stand-down names the per-write snapshot', () => { + const scope = flowCelRootScope({ nodes: [] }, [], undefined, new Map(), perWriteSnapshotEntrance()); + expect(scope.provable).toBe(false); + expect(scope.openedBy).toContain('per-write snapshot'); + }); +}); diff --git a/packages/lint/src/validate-expressions.test.ts b/packages/lint/src/validate-expressions.test.ts index bc826435f0a..812605ae001 100644 --- a/packages/lint/src/validate-expressions.test.ts +++ b/packages/lint/src/validate-expressions.test.ts @@ -312,7 +312,10 @@ describe('validateStackExpressions (ADR-0032 build-time)', () => { expect(issues).toHaveLength(0); }); - // #1928 tier 3 — a likely field typo in a flow condition is a non-blocking warning. + // #1928 tier 3 — a likely field typo in a flow condition is a non-blocking + // warning WHERE THE DOOR CANNOT TELL whether a flow variable answers to the + // name: here a `script` node's registered function gets the live variable + // map and may bind any name (#22565 stands down on such a flow). it('warns (severity=warning) on a likely field typo in a flow condition', () => { const issues = validateStackExpressions({ objects: [{ name: 'crm_opportunity', fields: { stage: { type: 'select' }, amount: { type: 'currency' } } }], @@ -320,6 +323,7 @@ describe('validateStackExpressions (ADR-0032 build-time)', () => { name: 'opp_won', nodes: [ { id: 'start', type: 'start', config: { objectName: 'crm_opportunity', condition: 'stagee == "closed_won"' } }, + { id: 'enrich', type: 'script', config: { function: 'enrich_opportunity' } }, ], edges: [], }], @@ -329,11 +333,35 @@ describe('validateStackExpressions (ADR-0032 build-time)', () => { expect(issues[0].message).toMatch(/did you mean `stage`/); }); - it('does not warn when the bare ref is far from any field (likely a flow variable)', () => { + // [#22565] RE-JUDGED: on a flow whose every binding the door can read, the + // same typo binds nothing at all, so the run fails `Unknown variable: + // stagee`. One finding, at `error`, still naming the field it is near — the + // near-miss warning is withdrawn, not doubled. + it('refuses (severity=error) the same typo on a flow whose bindings are all in hand, with the did-you-mean', () => { + const issues = validateStackExpressions({ + objects: [{ name: 'crm_opportunity', fields: { stage: { type: 'select' }, amount: { type: 'currency' } } }], + flows: [{ + name: 'opp_won', + nodes: [ + { id: 'start', type: 'start', config: { objectName: 'crm_opportunity', condition: 'stagee == "closed_won"' } }, + ], + edges: [], + }], + }); + expect(issues).toHaveLength(1); + expect(issues[0].severity).toBe('error'); + expect(issues[0].message).toMatch(/^`stagee` is not bound in this flow's expression scope/); + expect(issues[0].message).toMatch(/did you mean `stage`/); + }); + + it('does not warn when the bare ref is far from any field (a flow variable)', () => { const issues = validateStackExpressions({ objects: [{ name: 'crm_opportunity', fields: { stage: { type: 'select' } } }], flows: [{ name: 'renewal', + // [#22565] Declared: the run binds a flow variable only when the flow + // says so, and an undeclared `expiring_deals` faults it. + variables: [{ name: 'expiring_deals', type: 'list' }], nodes: [{ id: 'start', type: 'start', config: { objectName: 'crm_opportunity' } }], edges: [{ id: 'e1', source: 'start', target: 'end', condition: 'expiring_deals.length > 0' }], }], @@ -825,6 +853,8 @@ describe('validateStackExpressions (ADR-0032 build-time)', () => { objects: [{ name: 'crm_opportunity', fields: { amount: { type: 'currency' } } }], flows: [{ name: 'opp_flow', + // [#22565] Declared — see the `expiring_deals` fixture above. + variables: [{ name: 'expiring_count', type: 'number' }], nodes: [{ id: 'start', type: 'start', config: { objectName: 'crm_opportunity' } }], edges: [{ id: 'e1', source: 'start', target: 'end', condition: 'amount / 100 > 5 && expiring_count * 2 > 3' }], }], @@ -2598,6 +2628,10 @@ describe('validateStackExpressions (ADR-0032 build-time)', () => { objects, flows: [{ name: 'convert_lead', + // [#22565] The variable the branch reads, declared — app-crm binds it + // with a node; a flow that binds nothing named `lead_record` faults + // `Unknown variable: lead_record` on the branch, and is refused for it. + variables: [{ name: 'lead_record', type: 'object' }], nodes: [ { id: 'start', type: 'start', config: { objectName: 'crm_lead' } }, { id: 'check', type: 'decision', config: { conditions: [{ label: 'Yes', expression }] } }, @@ -3498,6 +3532,14 @@ describe('validateStackExpressions — reads only keys the spec declares (meta-t // Its one "key" is `Map.prototype.get`; the metadata key it is built // from is read off the tabled `obj` receiver (`obj.attachedOnRead`). 'attachedOnReadIndex', + // [#22565] The flow CEL root judge's findings for one site, one unbound + // root at a time. Their keys are that module's own `{ root, members }` and + // the array's own methods — never metadata keys. + 'unbound', 'unboundRoot', + // [#22565] The flow name → entrance index the root judge reads. Its one + // "key" is `Map.prototype.get`; the metadata keys it is built from are read + // in `flow-cel-root-scope.ts`, not here. + 'flowEntrances', ]); expect(receivers.filter((r) => !tabled.has(r) && !PLUMBING.has(r))).toEqual([]); }); @@ -4811,6 +4853,9 @@ describe('a blank string in a ledger predicate slot (#17493)', () => { const flowStack = (...middle: Record[]) => ({ flows: [{ name: 'blank_flow', + // [#22565] The variable the non-blank controls read, declared: an + // `amount` the flow does not bind is refused as an unbound root. + variables: [{ name: 'amount', type: 'number' }], nodes: [{ id: 'start', type: 'start' }, ...middle], edges: [], }], @@ -4903,6 +4948,9 @@ describe('a decision branch with no `expression` (#19961)', () => { const flowStack = (...branches: Record[]) => ({ flows: [{ name: 'absent_flow', + // [#22565] The variable the valid branch reads, declared — see the + // #17493 fixture above. + variables: [{ name: 'amount', type: 'number' }], nodes: [{ id: 'start', type: 'start' }, { id: 'check', type: 'decision', config: { conditions: branches } }], edges: [], }], diff --git a/packages/lint/src/validate-expressions.ts b/packages/lint/src/validate-expressions.ts index c404d97dbbc..62ad8d3a9fc 100644 --- a/packages/lint/src/validate-expressions.ts +++ b/packages/lint/src/validate-expressions.ts @@ -122,6 +122,14 @@ import { referenceCarrierOf, REFERENCE_VALUE_TYPES } from '@objectstack/spec/dat import { EvalUserSchema } from '@objectstack/spec/identity'; import { collectFlowVariableNames, shadowedFieldReads, shadowedFieldMessage } from './flow-variable-scope.js'; +import { + flowCelEntrances, + flowCelRootScope, + perWriteSnapshotEntrance, + unboundFlowCelRoots, + unboundFlowCelRootMessage, +} from './flow-cel-root-scope.js'; +import type { FlowCelEntrance } from './flow-cel-root-scope.js'; import { injectedColumnsFor, unprovisionedInjectedColumnsFor } from './system-fields.js'; import { findUnguardedNullableOperands, nullGuardMessage } from './validate-null-guards.js'; import type { NullGuardOutcome } from './validate-null-guards.js'; @@ -1156,6 +1164,24 @@ function isBareReferenceToAny(diagnostic: string, roots: readonly string[]): boo return roots.some((root) => diagnostic.startsWith(`bare reference \`${root}\``)); } +/** + * [#22565] Whether `diagnostic` is the flattened-scope near-miss WARNING + * `validateExpression` gives a bare non-field identifier on a flow condition + * ("`X` is not a field of `obj` — did you mean …? If `X` is a flow variable this + * is safe to ignore") about one of `roots`. + * + * The flow walk withdraws that warning at a site where the unbound-root judge + * refuses the same root: the judge has read the flow's whole bound set, so "if it + * is a flow variable" is already answered no, and one broken root earns one + * finding. Matched on the leading sentence for the reason + * {@link isBareReferenceToAny} gives; a reword upstream makes the withdrawal miss + * LOUDLY — the root then carries two findings and the `toHaveLength(1)` pin in + * `validate-expressions.flow-cel-root.test.ts` goes red. + */ +function isFlattenedNearMissFor(diagnostic: string, roots: readonly string[]): boolean { + return roots.some((root) => diagnostic.startsWith(`\`${root}\` is not a field of`)); +} + /** * [#20078] The field-level predicate slots a read THROUGH a reference field is * refused on: the two field-rule slots the server evaluates on a write, and a @@ -2111,9 +2137,13 @@ export function runStackExpressionPasses(stack: AnyRec, options: StackExpression * engine's `registerFlow` throw: build and run time must agree about what * registers, and a malformed envelope used to be stored verbatim and rendered * by `notify` as JSON with nothing said at any layer. + * + * @returns whether the slot was refused on SHAPE, so the caller can skip the + * root judgment (#22565) — a value that is not an envelope has no source to + * read roots from. */ - const checkDeclaredValue = (where: string, raw: unknown): void => { - if (raw == null) return; + const checkDeclaredValue = (where: string, raw: unknown): { refused: boolean } => { + if (raw == null) return { refused: false }; // `celSourceOf` — not a second read of `.source`: the same helper every // other slot in this file locates its finding with. const source = celSourceOf(raw) ?? ''; @@ -2121,7 +2151,7 @@ export function runStackExpressionPasses(stack: AnyRec, options: StackExpression if (!shape.success) { // Already prefixed by the spec's own refinement — do not say it twice. for (const issue of shape.error.issues) issues.push({ where, message: issue.message, source, severity: 'error' }); - return; + return { refused: true }; } const res = validateExpression('value', raw as { dialect?: string; source?: string }); for (const e of res.errors) { @@ -2130,9 +2160,20 @@ export function runStackExpressionPasses(stack: AnyRec, options: StackExpression for (const w of res.warnings) { issues.push({ where, message: w.message, source: w.source, severity: 'warning' }); } + return { refused: false }; }; // ── Flows ────────────────────────────────────────────────────────── + // [#22565] Which flows an entrance opens to the root judgment — read once per + // stack, because an entrance lives outside the flow it feeds (an action, + // another flow's `subflow` / `map` node). At the runtime publish gate + // (`runtimeWriteType` set) the per-write snapshot holds the written flow alone, + // so no entrance is visible there and the judgment stands down + // (`perWriteSnapshotEntrance`) until #22636 widens that snapshot; every other + // verdict this pass gives at that door is unchanged. + const runtimeGateWrite = options.runtimeWriteType !== undefined; + const flowEntrances: ReadonlyMap = + objectWrite || runtimeGateWrite ? new Map() : flowCelEntrances(stack, fieldIndex); for (const flow of objectWrite ? [] : recordsOf(stack.flows)) { const flowName = typeof flow.name === 'string' ? flow.name : '(unnamed flow)'; // `Array.isArray` proves the LIST, never its MEMBERS — the sentence #15742 @@ -2186,6 +2227,56 @@ export function runStackExpressionPasses(stack: AnyRec, options: StackExpression // depend on traversal order. `collectFlowGraphs` is still called once. const declaredVariables = collectFlowVariableNames(flow, graphs); + /** + * [#22565] The flow CEL ROOT judgment. A root the run does not bind fails + * that run with `Unknown variable: X`, and before this pass nothing at build + * said so: `user.id == "u1"` — the run-user spelling formulas, RLS and the + * client accept — validated clean and faulted at the first evaluation, + * because flow CEL binds the run's user as `current_user` only. + * + * One bound set per flow, built once like `declaredVariables` above and for + * the same reason (one variable map serves the whole run): the flow's own + * bindings, the engine's roots and every object's fields — see + * `flow-cel-root-scope.ts` for the set, and for the flows it stands down on + * because their run can bind a name the reader cannot see: a node's, or an + * entrance's (`flowEntrances` above). `error`: the run faults on the root, + * every time the expression is evaluated. + */ + const rootScope = flowCelRootScope( + flow, + graphs, + objectName, + fieldIndex, + runtimeGateWrite + ? perWriteSnapshotEntrance() + : flowEntrances.get(typeof flow.name === 'string' ? flow.name : '') ?? {}, + ); + + /** + * [#22565] Refuse each root `raw` reads that the flow does not bind. + * + * @param from the index in `issues` this site's own findings start at — a + * flattened-scope near-miss warning `check()` pushed there about a root + * refused here is withdrawn ({@link isFlattenedNearMissFor}). + * @param role a `value` slot's plain string is a literal, so only its CEL + * envelope is read; a predicate slot is CEL as a string too. + */ + const refuseUnboundRoots = (where: string, raw: unknown, from: number, role: 'predicate' | 'value'): void => { + if (role === 'value' && !(raw && typeof raw === 'object' && (raw as AnyRec).dialect === 'cel')) return; + const source = celSourceOf(raw); + if (!source) return; + const unbound = unboundFlowCelRoots(source, rootScope); + if (unbound.length === 0) return; + const refused = unbound.map((unboundRoot) => unboundRoot.root); + for (let i = issues.length - 1; i >= from; i--) { + const issue = issues[i]!; + if (issue.severity === 'warning' && isFlattenedNearMissFor(issue.message, refused)) issues.splice(i, 1); + } + for (const unboundRoot of unbound) { + issues.push({ where, message: unboundFlowCelRootMessage(unboundRoot, rootScope), source, severity: 'error' }); + } + }; + /** * [#14089] The one bare-identifier case a flattened flow scope may not stay * silent about: a name bound as BOTH a declared flow variable and a field on @@ -2275,8 +2366,10 @@ export function runStackExpressionPasses(stack: AnyRec, options: StackExpression const cfg = (node.config ?? {}) as AnyRec; const nodeCondWhere = `${at} · node '${node.id}' (${node.type}) condition`; if (!checkStructuralCondition(nodeCondWhere, cfg.condition).refused) { + const from = issues.length; check(nodeCondWhere, cfg.condition, objectName); warnShadowedFieldReads(nodeCondWhere, cfg.condition); + refuseUnboundRoots(nodeCondWhere, cfg.condition, from, 'predicate'); } // Descriptor-declared expression slots (#4027). Before this, the traversal @@ -2328,10 +2421,14 @@ export function runStackExpressionPasses(stack: AnyRec, options: StackExpression // and a value envelope is evaluated in the flow-variable scope by // design — there is no field reading to displace. if (found.entry.role === 'value') { - checkDeclaredValue(slotWhere, found.value); + const from = issues.length; + if (!checkDeclaredValue(slotWhere, found.value).refused) { + refuseUnboundRoots(slotWhere, found.value, from, 'value'); + } continue; } if (found.entry.role !== 'predicate') continue; + const slotFrom = issues.length; // [#15572] A slot refused on SHAPE gets no second diagnostic: the // shadowing warning is about which scope a CEL source resolves in, // and a value that is not CEL text has no source to resolve. @@ -2364,6 +2461,8 @@ export function runStackExpressionPasses(stack: AnyRec, options: StackExpression // moves no accept set and judges no bare identifier for being bare // (the 2026-09-01 option-C ruling's letter). warnShadowedFieldReads(slotWhere, found.value); + // [#22565] The same scope again, judged for roots the run cannot bind. + refuseUnboundRoots(slotWhere, found.value, slotFrom, 'predicate'); } // [#19939] The `{…}` template dialect is retired from the value slots // (the C half of #11182 ruling D, which this replaced: until then a @@ -2466,8 +2565,10 @@ export function runStackExpressionPasses(stack: AnyRec, options: StackExpression for (const edge of recordsOf(graph.edges)) { const edgeCondWhere = `${at} · edge '${edge.id}' (${edge.source}→${edge.target}) condition`; if (!checkStructuralCondition(edgeCondWhere, edge.condition).refused) { + const from = issues.length; check(edgeCondWhere, edge.condition, objectName); warnShadowedFieldReads(edgeCondWhere, edge.condition); + refuseUnboundRoots(edgeCondWhere, edge.condition, from, 'predicate'); } } // No `checkNullGuards` on node/edge conditions — and NOT for the reason 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 new file mode 100644 index 00000000000..eecfbcc5705 --- /dev/null +++ b/packages/spec/src/migrations/entries/semantic/18.flow-cel-unbound-root-refused.ts @@ -0,0 +1,49 @@ +// Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license. + +import type { SemanticMigration } from '../../types.js'; + +// 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, +// so an alias there failed every run that reached it while objectstack validate +// passed it. Semantic-only — none of these roots ever evaluated in a flow, so no +// D2 conversion has behaviour to carry over. +export const entry: SemanticMigration = { + id: 'flow-cel-unbound-root-refused', + // 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 whose root identifier the flow does not bind, ' + + 'such as user.id, ctx.user.id or os.user.id', + replacement: + 'current_user for the run\'s user: user.id, ctx.user.id and os.user.id become current_user.id, and a flow that ' + + 'can run without a user guards it as current_user != null ? current_user.id : null. Any other root becomes a ' + + 'name the flow binds — a declared variable, an outputVariable, an iterator, index or error variable, an ' + + 'assignment target, a node id or a screen field — or a field of the trigger record, read bare or as ' + + '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 ' + + '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 ' + + 'and os.user as well, and flow CEL does not, because a top-level user would collide with a variable or a ' + + 'lookup field of that name. objectstack validate now refuses such a root, naming current_user for the user ' + + 'spellings and the in-scope names for any other. A flow whose run can bind a name the reader cannot see is not ' + + 'judged: one with a script or connector_action node, a wait, subflow or map node a resume can fold a bag into, ' + + 'a screen with no field list, or a plugin node type; and one an entrance the stack declares hands a record ' + + 'whose keys are not in hand: a record trigger or time-relative sweep on an object the stack does not declare, ' + + 'an api trigger (the inbound hook hands the request body in as the record), a flow action on an undeclared ' + + 'object, a map node whose itemObject is undeclared or absent, or a subflow or map parent that is itself such a ' + + 'flow. The runtime publish gate does not give this verdict yet: it judges a flow write without the stack\'s ' + + 'actions and other flows, so it cannot see those entrances. No D2 conversion exists: none of these roots ever ' + + 'evaluated in a flow, so there is no behaviour to carry over, and a run without a user needs a guard only the ' + + 'author can choose.', + acceptanceCriteria: + 'Run objectstack validate: it reports each refused root as expression-invalid at the flow, the node or edge and ' + + 'the slot, naming the root and its remedy. Replace each run-user spelling with current_user, guarded where the ' + + 'flow can run without a user, and bind or correct every other root. Re-run the flow paths that evaluate those ' + + 'expressions and confirm each one evaluates instead of failing with an unknown-variable fault.', +}; diff --git a/packages/spec/src/migrations/registry.ts b/packages/spec/src/migrations/registry.ts index 4d51085210b..830c1cbb569 100644 --- a/packages/spec/src/migrations/registry.ts +++ b/packages/spec/src/migrations/registry.ts @@ -5697,6 +5697,20 @@ const STEP18_RATIONALE: readonly RationaleFragment[] = [ + 'value the author meant. Its D3 record is the semantic entry ' + '`flow-builtin-node-config-values-refused`.', }, + { + id: 'flow-cel-unbound-root-refused', + order: 92, + text: + 'It also refuses, at `objectstack validate`, a flow CEL root the flow does not bind — in a trigger gate, an ' + + 'edge, a `decision` branch, a screen field `visibleWhen` or a CEL value envelope. Such a root fails every run ' + + 'that reaches it with an unknown-variable fault, and the build used to pass it. The run-user spellings ' + + 'formulas, row-level security and the client accept (`user`, `ctx.user`, `os.user`) are the reachable case: ' + + 'flow CEL binds the run\'s user as `current_user` only, and the refusal names it. A flow whose run can bind a ' + + 'name the reader cannot see is left unjudged — through a node, or through an entrance the stack declares that ' + + 'hands it a record whose keys are not in hand — and so, for now, is every flow write at the runtime publish ' + + 'gate, whose snapshot carries no actions or other flows. No D2 conversion exists: none of these roots ever ' + + 'evaluated in a flow. Its D3 record is the semantic entry `flow-cel-unbound-root-refused`.', + }, { id: 'flow-decision-edge-branching-first-match', order: 45, @@ -13777,6 +13791,51 @@ 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 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, + // so an alias there failed every run that reached it while objectstack validate + // passed it. Semantic-only — none of these roots ever evaluated in a flow, so no + // D2 conversion has behaviour to carry over. + { + id: 'flow-cel-unbound-root-refused', + // 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 whose root identifier the flow does not bind, ' + + 'such as user.id, ctx.user.id or os.user.id', + replacement: + 'current_user for the run\'s user: user.id, ctx.user.id and os.user.id become current_user.id, and a flow that ' + + 'can run without a user guards it as current_user != null ? current_user.id : null. Any other root becomes a ' + + 'name the flow binds — a declared variable, an outputVariable, an iterator, index or error variable, an ' + + 'assignment target, a node id or a screen field — or a field of the trigger record, read bare or as ' + + '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 ' + + '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 ' + + 'and os.user as well, and flow CEL does not, because a top-level user would collide with a variable or a ' + + 'lookup field of that name. objectstack validate now refuses such a root, naming current_user for the user ' + + 'spellings and the in-scope names for any other. A flow whose run can bind a name the reader cannot see is not ' + + 'judged: one with a script or connector_action node, a wait, subflow or map node a resume can fold a bag into, ' + + 'a screen with no field list, or a plugin node type; and one an entrance the stack declares hands a record ' + + 'whose keys are not in hand: a record trigger or time-relative sweep on an object the stack does not declare, ' + + 'an api trigger (the inbound hook hands the request body in as the record), a flow action on an undeclared ' + + 'object, a map node whose itemObject is undeclared or absent, or a subflow or map parent that is itself such a ' + + 'flow. The runtime publish gate does not give this verdict yet: it judges a flow write without the stack\'s ' + + 'actions and other flows, so it cannot see those entrances. No D2 conversion exists: none of these roots ever ' + + 'evaluated in a flow, so there is no behaviour to carry over, and a run without a user needs a guard only the ' + + 'author can choose.', + acceptanceCriteria: + 'Run objectstack validate: it reports each refused root as expression-invalid at the flow, the node or edge and ' + + 'the slot, naming the root and its remedy. Replace each run-user spelling with current_user, guarded where the ' + + 'flow can run without a user, and bind or correct every other root. Re-run the flow paths that evaluate those ' + + 'expressions and confirm each one evaluates instead of failing with an unknown-variable fault.', + }, // The absent half of the decision-branch predicate rule. A SEPARATE entry from // `flow-predicate-slot-blank-string-refused` on purpose: that one keeps the // run a blank predicate made (it evaluated `false`, so `'false'` runs the same