diff --git a/.changeset/19939-flow-crud-filter-operands.md b/.changeset/19939-flow-crud-filter-operands.md new file mode 100644 index 00000000000..c6f671215f6 --- /dev/null +++ b/.changeset/19939-flow-crud-filter-operands.md @@ -0,0 +1,53 @@ +--- +'@objectstack/spec': major +'@objectstack/service-automation': major +'@objectstack/lint': major +--- + +Each operand of a `get_record`, `update_record` or `delete_record` node's `filter` is a value slot now. A field's value, an operator's comparand and each element of a list comparand take a CEL value envelope, `{ id: { dialect: 'cel', source: 'record.id' } }`, which the executor evaluates in the run's scope before the query, or a literal handed to the query as written. A query placeholder — `{current_user_id}`, `{current_org_id}`, `{today}`, `{30_days_ago}` and the rest of the date macros — is such a literal: the query engine resolves it, and it passes untouched, as before. Every other single-brace `{…}` token at an operand is refused with its envelope: at `objectstack validate`, at `registerFlow`, at the metadata save door and by the executor's parse. An operand envelope that evaluates to `null` refuses the node, as a token that resolved to nothing did. + +Clause-②: yes (narrowing: a flow {…} token at a get_record, update_record or delete_record filter operand is refused, a whole-string query placeholder excepted, an object naming a string dialect at an operand must be a valid CEL value envelope, an operand envelope that evaluates to null refuses the node, and @objectstack/lint no longer exports validateFlowFilterTokens, FLOW_FILTER_TOKEN_UNKNOWN or FlowFilterTokenFinding and retires the flow-filter-token-unknown rule id; widening: a filter operand gains the CEL value envelope arm, the exported FlowNodeExpressionPath type gains the optional filterOperands field, ValueSlotTemplateOptions gains filterOperand, FLOW_NODE_EXPRESSION_PATHS gains the three filter rows, LEDGER_DECLARED_NODE_CONFIG_SCHEMAS gains get_record and delete_record, the published GetRecordConfig, UpdateRecordConfig and DeleteRecordConfig JSON Schemas mark filter as a value slot, and flowFilterOperands, FlowFilterOperand, classifyFlowFilterOperandString and FlowFilterOperandStringKind are new exports) + + + +**BREAKING**: an accept-set narrowing on a published authoring surface, shipped as `major` on the v18 line (`.changeset/pre.json` is open on `main` in `next` pre mode, so the release is `18.0.0-next.*`). + +**Why.** ADR-0032 Decision 2 makes a computed value whole-field CEL — its contract table names "filter values" — and Decision 3 deletes the single brace. A CRUD filter was the last value position that read the single-brace dialect: the executor interpolated the whole filter, resolving a flow token against the run's variables and handing a token no variable resolved on to the query engine when it was a query placeholder. Two `{…}` vocabularies met in one string. The query placeholders are the query-filter surface, which ADR-0032 puts out of its scope, so they stay; the flow token goes. + +**One classifier.** `classifyFlowFilterOperandString` (`@objectstack/spec/automation`) draws the line: a string that is one whole wrapped token the query engine resolves on the server (`isKnownFilterToken`) is a placeholder and passes; any other string carrying a `{…}` token is refused; the rest is a literal. The node contracts, `registerFlow`, `objectstack validate` and the executor's parse all read it, through the value-slot judge. + +**What an operand is.** The operands are the positions a value sits at, read the way the filter grammar reads a `FilterCondition` (`flowFilterOperands`): a field's value, each entry of an operator map (`$gt`, `$in`, `$between`, …), each element of a list comparand, inside `$and`, `$or` and `$not` and inside a nested-relation condition. An envelope there is one operand, and may evaluate to a list (`{ id: { $in: { dialect: 'cel', source: 'record.ids' } } }`). The filter's keys, operators and combinators are its structure, never evaluated. The top-level `filter` is never an operand. + +**What changes where a value may be absent.** The template erased a condition whose token resolved to nothing, and the node was refused. Under CEL an absent variable or key fails the run before any query, naming it, and an envelope that evaluates to `null` — a guarded form, a field that holds no value — refuses the node with the guard's message: a condition whose comparand is `null` or nothing does not narrow a query, it widens it. So route around the node with a `decision` where the value may be absent, rather than guarding it to `null`. To match an empty field, write the literal `null` (or `{ $null: true }`), never a computed one. + +**A flow variable named like a placeholder no longer shadows it.** Under the template a flow variable named `today` or `current_user_id` won over the placeholder of the same name. No flow token is read at an operand now, so the placeholder is always the query engine's; read the variable with an envelope, `{ dialect: 'cel', source: 'today' }`. No flow, variable or output variable in this repository is named like a placeholder. + +**A related field named `dialect`.** An object at a field's value that names a string `dialect` is read as an envelope. A nested-relation condition on a related object's field named `dialect` is written through an operator map, `{ rel: { dialect: { $eq: 'x' } } }`. No object in this repository declares such a field. + +**The lint rule `flow-filter-token-unknown` retires.** It reported a `{FUNC()}` token neither the flow template dialect nor the query placeholders define at a flow filter. Every token it reported is refused now by the value-slot judge at the same position and severity, with the envelope that replaces it, so the rule, its `os explain` entry and its exports (`validateFlowFilterTokens`, `FLOW_FILTER_TOKEN_UNKNOWN`, `FlowFilterTokenFinding`) are removed rather than left reporting the same defect a second time with the retired dialect's remedy. + +## FROM → TO + +| you wrote | write instead | what changes | +|:--|:--|:--| +| `filter: { id: '{record.id}' }` | `filter: { id: { dialect: 'cel', source: 'record.id' } }` | an absent `record.id` fails the run before the query instead of refusing the node | +| `filter: { amount: { $gt: '{record.cap}' } }` | `filter: { amount: { $gt: { dialect: 'cel', source: 'record.cap' } } }` | a `null` cap refuses the node, as before | +| `filter: { id: { $in: ['{a}', '{b}'] } }` | `filter: { id: { $in: [{ dialect: 'cel', source: 'a' }, { dialect: 'cel', source: 'b' }] } }`, or one envelope evaluating to the list | — | +| `filter: { owner: '{$User.Id}' }` | `filter: { owner: { dialect: 'cel', source: 'current_user.id' } }` | in a run with no user `current_user` is `null` and the read fails: skip the node with a `decision` on `current_user != null` | +| `filter: { due: { $lt: '{TODAY() - 3}' } }` | `filter: { due: { $lt: { dialect: 'cel', source: 'isoDate(daysAgo(3))' } } }` | none for a whole number of days | +| `filter: { owner: '{current_user_id}' }`, `{ due: { $lt: '{today}' } }` | unchanged | a query placeholder stays the query engine's | +| `filter: { owner: '{today}' }` meaning a flow variable named `today` | `filter: { owner: { dialect: 'cel', source: 'today' } }` | the placeholder no longer shadows the variable | + +**The one-line fix: write each filter operand the flow computes as a CEL value envelope, and keep the query placeholders as they are.** + +**Who is affected, measured.** In this repository, at the base: seven flow tokens at a filter operand in the shipped examples (`app-crm` 2, `app-showcase` 3, `app-todo` 2 — `app-todo`'s two query placeholders stay), five in the docs (`getting-started/common-patterns`, `kernel/runtime-services/examples` 2, the `service-automation` README 2), the flows guide's and the spec's own remedy texts (a `$User.Email` read named `filter: { id: '{uid}' }`; a `recordId` refusal named `filter: { id: '{record.id}' }`) and the `FlowSchema` example; all are migrated in this change. The value-slot retirement's run-user changeset on this line names `filter: { id: '{uid}' }` for a `$User.Email` read; this one supersedes that line: read the user record with `filter: { id: { dialect: 'cel', source: 'current_user.id' } }`. hotcrm was not measured. + +**Still accepted, unchanged.** Every literal operand, every query placeholder, a filter's keys, operators and combinators, an empty filter, and a filter on `create_record`, which has none. The `filters` alias still converts to `filter` at load. + +### The kit + +- **The contract.** `GetRecordConfigSchema`, `UpdateRecordConfigSchema` and `DeleteRecordConfigSchema` judge each filter operand: an envelope must be a valid CEL value envelope, and a `{…}` token in a literal is refused by the value-slot judge in its filter mode, naming the operand's envelope and what `null` does there. Each `filter`'s `describe()` and TSDoc is restated, and so are the `recordId` guidance and the run-user remedy, which named a single-brace filter value. +- **The ledger.** `FLOW_NODE_EXPRESSION_PATHS` gains a `filter` row per node, with the new `filterOperands` flag: both walks emit each operand at its own path (`filter.amount.$gt`, `filter.id.$in[0]`), never the filter itself. `GetRecordConfigSchema` and `DeleteRecordConfigSchema` join `LEDGER_DECLARED_NODE_CONFIG_SCHEMAS`. +- **The executor.** The CRUD executors parse their config as authored, evaluate each operand envelope through `AutomationEngine.evaluateValueEnvelope` before the query, hand every other operand on as written, and refuse the node — as a guard a `fault` edge cannot route — when an operand evaluates to `null`. The single-brace `interpolateFilter` is deleted. +- **The lint.** `objectstack validate` refuses a token at each operand position and judges an operand envelope as CEL through the shared judges; the wrong-syntax hints at a filter operand name its envelope; `flow-filter-token-unknown` retires. +- **ADR-0087.** The step-18 D3 entry `flow-value-slot-template-dialect-refused` is amended. No key is removed, so there is no tombstone, and there is no D2 conversion. diff --git a/content/docs/automation/flows.mdx b/content/docs/automation/flows.mdx index df780ea7c8d..f9e4b12f869 100644 --- a/content/docs/automation/flows.mdx +++ b/content/docs/automation/flows.mdx @@ -283,7 +283,7 @@ for some input, so the rewrite is yours to judge. | `'Follow up on {record.name}'` | `source: "'Follow up on ' + record.name"` | wrap a non-string hole in `string(…)`, one that may be null in `coalesce(…, '')` | | braces meant literally, `'{"a": 1}'` | `source: "'{\"a\": 1}'"` | a CEL string literal | | `'{$User.Id}'` | `source: 'current_user.id'` | `current_user` is the run's user, and `null` in a run with none (a schedule, a record change made by a system write), where this read fails the run. In a flow that can run without a user, write `current_user != null ? current_user.id : null`: it writes `null` where the template wrote nothing, which on `update_record` clears a stored value the template left alone — or skip the node on `current_user != null` | -| `'{$User.Email}'`, `'{$User.Name}'`, any other `$User` path | read the user record by `current_user.id`: an `assignment` (`uid: { dialect: 'cel', source: 'current_user.id' }`), a `get_record` on `sys_user` with `filter: { id: '{uid}' }` and `outputVariable: 'me'`, then `source: 'me.email'` | these never resolved in any shipped run — they wrote nothing. `current_user` carries only what the run holds: `id`, `positions`, `organizationId`, `isPlatformAdmin` | +| `'{$User.Email}'`, `'{$User.Name}'`, any other `$User` path | read the user record by `current_user.id`: a `get_record` on `sys_user` with `filter: { id: { dialect: 'cel', source: 'current_user.id' } }` and `outputVariable: 'me'`, then `source: 'me.email'` | these never resolved in any shipped run — they wrote nothing. `current_user` carries only what the run holds: `id`, `positions`, `organizationId`, `isPlatformAdmin` | | `'{TODAY()}'` | `source: 'isoDate(today())'` | none: `isoDate` writes the `YYYY-MM-DD` text the macro wrote, on the UTC calendar | | `'{TODAY() + 7}'`, `'{TODAY() - 3}'` | `source: 'isoDate(daysFromNow(7))'`, `source: 'isoDate(daysAgo(3))'` | none for a whole number of days. A fraction (`'{TODAY() - 1.5}'`) is not: the template truncated the day of the month plus the offset, `addDays(today(), -1.5)` truncates the offset, so going back they land a day apart on most days of a month, and `daysAgo(1.5)` is refused at build. Write the whole number of days you mean | | `'{NOW()}'` | `source: 'isoDatetime(now())'` | none: `isoDatetime` writes the `YYYY-MM-DDTHH:mm:ss.sssZ` text the macro wrote | @@ -355,11 +355,41 @@ there — write the value itself (`sourceObject: 'crm_case'`). A `{…}` token there is refused, since it would reach the messaging service with its braces, and so is an envelope. The node's `actionUrl` is a text slot (below). +**A CRUD node's filter: each operand computes before the query, and the query +placeholders stay.** In a `get_record`, `update_record` or `delete_record` +`filter`, each **operand** — a field's value, an operator's comparand, each +element of a list comparand — is a value slot; the filter's keys, operators, +`$and` / `$or` / `$not` and nested-relation conditions are its structure: + +```ts +filter: { + id: { dialect: 'cel', source: 'record.id' }, // was '{record.id}' + amount: { $gt: { dialect: 'cel', source: 'record.cap * 2' } }, // a comparand + owner: { $in: ['usr_1', { dialect: 'cel', source: 'record.owner' }] }, + due_date: { $lt: '{today}' }, // a query placeholder: unchanged +} +``` + +The envelope is evaluated in the run's scope before the query is sent. A +**query placeholder** — `{current_user_id}`, `{current_org_id}`, `{today}`, +`{30_days_ago}` and the rest of the date macros — is the query engine's own +vocabulary: it is handed on untouched and the query engine resolves it, as +before. Every other `{…}` token there is refused with its envelope. A flow +variable that shares a placeholder's name no longer shadows it: the placeholder +is the query engine's, and the variable is read with an envelope. An operand +envelope that evaluates to `null` **refuses the node**, as a token that resolved +to nothing did: an absent condition does not narrow a query, it widens it, so +route around the node with a `decision` where the value may be absent rather +than guarding it to `null`. To match an empty field, write the literal `null` +(or `{ $null: true }`). A related field named `dialect` is matched through an +operator map, `{ dialect: { $eq: 'x' } }`, since an object naming a string +`dialect` at an operand is read as an envelope. + **Where an envelope is data, the remedy names none there.** An envelope evaluates only as the top-level value of a value slot — `fields`, `assignments`, a callee's `input` / `inputs`, a screen's `defaults`, -`defaultValue` or `recordId`, a `collection`, or a `notify` value (each entry of -a `recipients` list among them). Inside an object or list value, and in the two legacy `assignment` shapes +`defaultValue` or `recordId`, a `collection`, a `notify` value (each entry of +a `recipients` list among them), or a CRUD `filter` operand. Inside an object or list value, and in the two legacy `assignment` shapes (an `assignments: [{ variable, value }]` array, variables as the config's own keys), it is stored as the object it spells — so the refusal there names what does evaluate: @@ -1769,8 +1799,8 @@ A failure is a guard when both hold: re-running the flow unchanged could never succeed, **and** the fix is to edit metadata. In practice that is a missing required config key (a data node with no `objectName`, an `http` node with no `url`, a `subflow` or `map` with no `flowName`, a `connector_action` with no -`connectorId`/`actionId`), a filter token that resolved to nothing so the -condition was dropped from the query, a graph that recurses past the nesting +`connectorId`/`actionId`), a filter operand that evaluated to nothing (`null`), +so the condition would have widened the query, a graph that recurses past the nesting ceiling, or a run that would execute unscoped. The split is deliberate. A dropped filter condition does not narrow a query, it @@ -2110,9 +2140,9 @@ CEL, and every computed value (a field value, an assignment value) is a CEL value envelope. **Text** slots — a `notify` title, message or link, an `http` url and header values, a screen title or description, a refusing `end` node's message — are `{{ }}` templates -(see [text slots](#text-slots-read-double-brace-holes)). Single braces remain -only on the value-like position that hands a resolved value over and that the -retirement has not reached yet, a CRUD `filter` value. +(see [text slots](#text-slots-read-double-brace-holes)). A CRUD `filter` +operand is a value too: a CEL value envelope, or a literal — a query +placeholder such as `{today}` among them, which the query engine resolves. | Where | Dialect | Write it like | Bindings | |:---|:---|:---|:---| @@ -2121,6 +2151,7 @@ retirement has not reached yet, a CRUD `filter` value. | Decision-node `conditions[].expression` | **CEL** (bare, no braces) | `order_amount > 10000` | flow variables by name, `vars.*` and `current_user` | | Field values and assignment values, as a **literal** | none — written as it is | `'open'`, `42`, `true`, `['a', 'b']` | — (a `{…}` template token is refused here since [#19939](https://github.com/objectstack-ai/objectstack/issues/19939), `{$User.*}` and the date macros `{NOW()}` / `{TODAY() ± N}` included: a date is `isoDate(today())` / `isoDatetime(now())` in an envelope) | | Field values and assignment values, as a **CEL value envelope** | **CEL** (in an envelope) | `{ dialect: 'cel', source: 'round(price * 100.0) / 100.0' }` | flow variables by name, `vars.*` and `current_user` — the whole CEL stdlib (`joinNonEmpty`, …) | +| A CRUD `filter` operand — a field's value, an operator's comparand, a list comparand's element | **CEL** (in an envelope), or a literal — a query placeholder (`{current_user_id}`, `{today}`) included | `{ id: { dialect: 'cel', source: 'record.id' }, due_date: { $lt: '{today}' } }` | flow variables by name, `vars.*` and `current_user`; the placeholders are the query engine's | | Text slots — `notify` `title` / `message` / `actionUrl`, `http` `url` and `headers` values, `screen` `title` / `description`, `end` `message` | **template** (`{{ }}` holes) | `'Deal won: {{ record.name }} ({{ record.amount \| currency }})'` | flow variables by name (`$`-named ones too: `{{ $error.message }}`) — a path and an optional formatter, never logic | A value slot takes either form, chosen by shape: an object naming a `dialect` @@ -2153,14 +2184,10 @@ and is read as `vars["current_user"]` / `vars["vars"]`. braces. 3. **A `{…}` template in a value slot** — `total: '{round(x * 100) / 100}'`, `owner: '{record.owner}'` — is refused with the CEL spelling to write - instead. In a single-brace position (a CRUD `filter` value), an - unsupported function in braces — `'{ROUND(x, 2)}'`, `'{Math.round(x)}'`, - `'{(x).toFixed(2)}'` — fails the node with a **named error** listing the - supported set and, where one is close, the spelling you meant. Before #11060 - this was a *silent* failure: the unknown name was rewritten to `null` and the - field was simply written `undefined`. A `fault` edge does not catch this - error — the expression itself is wrong, so re-running can never succeed; fix - the spelling. + instead, a CRUD `filter` operand included (a query placeholder such as + `{today}` there stays). An unknown function in the envelope — + `round(x, 2)`, `Math.round(x)` — is refused by the envelope's CEL check, + with the spelling you meant where one is close. 4. **A single-brace token in a text slot** — `title: 'Deal won: {record.name}'` — is refused with its `{{ }}` spelling ([text slots](#text-slots-read-double-brace-holes)): it is no placeholder there, and would go out as literal text. diff --git a/content/docs/deployment/cli.mdx b/content/docs/deployment/cli.mdx index 7ec06a1ba69..1636a0e3259 100644 --- a/content/docs/deployment/cli.mdx +++ b/content/docs/deployment/cli.mdx @@ -675,7 +675,7 @@ os compile --json # JSON output for CI pipelines → Normalizing stack definition... → Lowering inline handlers... → Validating protocol compliance... - → Running author-time rules (50)... + → Running author-time rules (49)... → Checking that every required capability has a provider installable in this edition... → Collecting package docs (ADR-0046)... 0 collected → Writing artifact... diff --git a/content/docs/deployment/validating-metadata.mdx b/content/docs/deployment/validating-metadata.mdx index b4632a5b62c..1131c999b80 100644 --- a/content/docs/deployment/validating-metadata.mdx +++ b/content/docs/deployment/validating-metadata.mdx @@ -721,7 +721,7 @@ A clean run walks the registry and reports timing: Config: /path/to/support-desk/objectstack.config.ts Load time: 21ms → Validating against ObjectStack Protocol... - → Running author-time rules (50)... + → Running author-time rules (49)... → Checking that every required capability has a provider installable in this edition... → Checking package docs (ADR-0046)... diff --git a/content/docs/getting-started/build-with-claude-code.mdx b/content/docs/getting-started/build-with-claude-code.mdx index 81253c3c8e8..9cbd058a35e 100644 --- a/content/docs/getting-started/build-with-claude-code.mdx +++ b/content/docs/getting-started/build-with-claude-code.mdx @@ -302,7 +302,7 @@ visible: 'status != "resolved"' ◆ Validate ──────────────────────────────────────── → Validating against ObjectStack Protocol... - → Running author-time rules (50)... + → Running author-time rules (49)... ✗ Author-time rules failed (1 issue) • stack · action 'resolve_ticket' visible: bare reference `status` — a diff --git a/content/docs/getting-started/common-patterns.mdx b/content/docs/getting-started/common-patterns.mdx index 7f4021d573f..edc92c7c1d5 100644 --- a/content/docs/getting-started/common-patterns.mdx +++ b/content/docs/getting-started/common-patterns.mdx @@ -348,7 +348,7 @@ Define a multi-step approval flow for records. label: 'Auto Approve', config: { objectName: 'expense_report', - filter: { id: '{record.id}' }, + filter: { id: { dialect: 'cel', source: 'record.id' } }, fields: { status: 'approved' } } }, diff --git a/content/docs/kernel/runtime-services/examples.mdx b/content/docs/kernel/runtime-services/examples.mdx index 0baf903b346..b8fd6eed89a 100644 --- a/content/docs/kernel/runtime-services/examples.mdx +++ b/content/docs/kernel/runtime-services/examples.mdx @@ -70,7 +70,7 @@ export const RollUpOrderTotals = defineFlow({ label: 'Read the order lines', config: { objectName: 'sales_order_line', - filter: { sales_order_id: '{record.id}' }, + filter: { sales_order_id: { dialect: 'cel', source: 'record.id' } }, fields: ['amount'], limit: 200, outputVariable: 'lines', @@ -92,7 +92,7 @@ export const RollUpOrderTotals = defineFlow({ label: 'Write the totals back', config: { objectName: 'sales_order', - filter: { id: '{record.id}' }, + filter: { id: { dialect: 'cel', source: 'record.id' } }, // A value slot computes with a CEL value envelope; the script's // output `totals` is bound by the node before this one. fields: { diff --git a/content/docs/references/automation/builtin-node-config.mdx b/content/docs/references/automation/builtin-node-config.mdx index 4bb6a1e4493..abd2519303d 100644 --- a/content/docs/references/automation/builtin-node-config.mdx +++ b/content/docs/references/automation/builtin-node-config.mdx @@ -40,7 +40,8 @@ parse the RAW stored config — their typed slots are strings (or `unknown` where values interpolate), so `{token}` templates pass and resolve at the executor's existing interpolation points. The value slots are the exception since #19939: the `{token}` dialect is retired there (the "value slots" -section below). So are the text slots since #22110: a screen `title` / +section below), each operand of the CRUD `filter` included, where a query +placeholder stays the query engine's. So are the text slots since #22110: a screen `title` / `description` and an `end` `message` render `{{ }}` holes, and a `{token}` there is refused (`flow-text-slot-template.ts`). @@ -80,7 +81,11 @@ a field value is a CEL value envelope or a literal — a `{token}` template is refused there since #19939 — and those maps are the expression ledger's `value`-role slots, as are, since #19939, the three maps a node hands to a callee, a `screen`'s `defaults.*`, field `defaultValue` and `recordId`, and -the list a `loop` and a `map` iterate (`FlowCollectionSlotSchema`). +the list a `loop` and a `map` iterate (`FlowCollectionSlotSchema`), and each +operand of the CRUD `filter` — a field's value, an operator's comparand, each +element of a list comparand — which a CEL value envelope computes before the +query while a query placeholder (`{current_user_id}`, `{today}`) stays the +query engine's (the "CRUD quartet" section below). Deliberately absent: - `decision` / `script` / `subflow` / `wait` / `connector_action` — the @@ -162,7 +167,7 @@ Value the variable takes: a CEL value envelope `{ dialect: 'cel', source }` eval | Property | Type | Required | Description | | :--- | :--- | :--- | :--- | | **objectName** | `string` | ✅ | Object to delete from | -| **filter** | `Record` | optional | Field/value pairs identifying the record(s) to delete | +| **filter** | `Record` | optional | Field/value pairs identifying the record(s) to delete: each operand — a field's value, an operator's comparand (`{"$ne": null}`), a list comparand's element — is a CEL value envelope `{ dialect: 'cel', source }` evaluated before the query (one that evaluates to null refuses the node), a query placeholder such as `{current_user_id}` or `{today}` the query engine resolves, or a literal; a flow `{…}` template token is refused | | **multi** | `boolean` | optional | Declare bulk intent: delete every row the filter matches (default false — a predicate delete without it is refused by the engine) | @@ -226,7 +231,7 @@ A value: a CEL value envelope `{ dialect: 'cel', source }` evaluated by the expr | Property | Type | Required | Description | | :--- | :--- | :--- | :--- | | **objectName** | `string` | ✅ | Object to query | -| **filter** | `Record` | optional | Field/value pairs to match (operator values like `{"$ne": null}` are preserved) | +| **filter** | `Record` | optional | Field/value pairs to match: each operand — a field's value, an operator's comparand (`{"$ne": null}`), a list comparand's element — is a CEL value envelope `{ dialect: 'cel', source }` evaluated before the query (one that evaluates to null refuses the node), a query placeholder such as `{current_user_id}` or `{today}` the query engine resolves, or a literal; a flow `{…}` template token is refused | | **fields** | `string[]` | optional | Field projection — only these fields are read (default: all) | | **limit** | `number` | optional | Max records to return; >1 switches to a multi-record query | | **outputVariable** | `string` | optional | Flow variable the result is bound to — a name without a leading `$` (the `$` names are the flow engine's own), read as `{{ name }}` | @@ -323,7 +328,7 @@ A value: a CEL value envelope `{ dialect: 'cel', source }` evaluated by the expr | Property | Type | Required | Description | | :--- | :--- | :--- | :--- | | **objectName** | `string` | ✅ | Object to update | -| **filter** | `Record` | optional | Field/value pairs identifying the record(s) to update | +| **filter** | `Record` | optional | Field/value pairs identifying the record(s) to update: each operand — a field's value, an operator's comparand (`{"$ne": null}`), a list comparand's element — is a CEL value envelope `{ dialect: 'cel', source }` evaluated before the query (one that evaluates to null refuses the node), a query placeholder such as `{current_user_id}` or `{today}` the query engine resolves, or a literal; a flow `{…}` template token is refused | | **fields** | `Record` | optional | Field values to write: each key is a field name, each value a CEL value envelope or a literal | | **multi** | `boolean` | optional | Declare bulk intent: update every row the filter matches (default false — a predicate update without it is refused by the engine) | diff --git a/content/docs/ui/react-pages.mdx b/content/docs/ui/react-pages.mdx index bff33aa7ab9..706043398f0 100644 --- a/content/docs/ui/react-pages.mdx +++ b/content/docs/ui/react-pages.mdx @@ -389,7 +389,7 @@ objectstack validate ──────────────────────────────────────── → Loading configuration... → Validating against ObjectStack Protocol... - → Running author-time rules (50)... + → Running author-time rules (49)... → Checking that every required capability has a provider installable in this edition... → Checking package docs (ADR-0046)... diff --git a/examples/app-crm/src/flows/convert-lead.flow.ts b/examples/app-crm/src/flows/convert-lead.flow.ts index 76dfda2f966..283bf1f9057 100644 --- a/examples/app-crm/src/flows/convert-lead.flow.ts +++ b/examples/app-crm/src/flows/convert-lead.flow.ts @@ -60,8 +60,9 @@ export const ConvertLeadScreenFlow = defineFlow({ objectName: 'crm_lead', // get_record filters via `filter` (it ignores a bare `recordId`), so // load THIS lead by id — otherwise findOne with an empty filter returns - // the first lead in the table. - filter: { id: '{recordId}' }, + // the first lead in the table. A filter operand the flow computes is a + // CEL value envelope, evaluated before the query. + filter: { id: { dialect: 'cel', source: 'recordId' } }, outputVariable: 'lead_record', }, }, @@ -150,7 +151,7 @@ export const ConvertLeadScreenFlow = defineFlow({ label: 'Mark Lead Converted', config: { objectName: 'crm_lead', - filter: { id: '{recordId}' }, + filter: { id: { dialect: 'cel', source: 'recordId' } }, // A value slot computes with a CEL value envelope — the `{…}` template // dialect is retired there. Both ids are bound by the screens above // (`idVariable`) before this node runs, so no `has()` guard is needed. diff --git a/examples/app-showcase/src/automation/flows/index.ts b/examples/app-showcase/src/automation/flows/index.ts index 588526b56d0..e2bc16dc156 100644 --- a/examples/app-showcase/src/automation/flows/index.ts +++ b/examples/app-showcase/src/automation/flows/index.ts @@ -122,7 +122,7 @@ export const ReassignWizardFlow = defineFlow({ label: 'Apply Reassignment', config: { objectName: 'showcase_task', - filter: { id: '{recordId}' }, + filter: { id: { dialect: 'cel', source: 'recordId' } }, // A CEL value envelope — the `{…}` template dialect is retired from // value slots. `new_assignee` is a required screen field, so it is // always bound here. @@ -1255,7 +1255,7 @@ export const ResilientSyncFlow = defineFlow({ label: 'Flag Sync Failure', config: { objectName: 'showcase_task', - filter: { id: '{record.id}' }, + filter: { id: { dialect: 'cel', source: 'record.id' } }, // `$error` (this try_catch's `errorVariable`) is not a CEL // identifier, so the envelope reads it through `vars`. fields: { sync_status: 'failed', sync_error: { dialect: 'cel', source: 'vars["$error"].message' } }, @@ -1368,7 +1368,7 @@ export const InvoiceDualSignoffFlow = defineFlow({ label: 'Flag: Held', config: { objectName: 'showcase_invoice', - filter: { id: '{record.id}' }, + filter: { id: { dialect: 'cel', source: 'record.id' } }, fields: { status: 'draft' }, }, }, diff --git a/examples/app-showcase/test/predicate-write-bulk-intent.test.ts b/examples/app-showcase/test/predicate-write-bulk-intent.test.ts index 0bf2f0b8633..1cc0b57a2ab 100644 --- a/examples/app-showcase/test/predicate-write-bulk-intent.test.ts +++ b/examples/app-showcase/test/predicate-write-bulk-intent.test.ts @@ -47,7 +47,7 @@ */ import { describe, it, expect } from 'vitest'; -import { DeleteRecordConfigSchema, UpdateRecordConfigSchema } from '@objectstack/spec/automation'; +import { DeleteRecordConfigSchema, UpdateRecordConfigSchema, isExpressionEnvelopeShaped } from '@objectstack/spec/automation'; import { allFlows } from '../src/automation/flows/index.js'; @@ -114,16 +114,18 @@ for (const flow of allFlows as unknown as FlowLike[]) { * requires — a SCALAR `id`? `{ id: { $in: [...] } }` does not qualify (the * engine refuses it), and neither does any other predicate. * - * `{recordId}` / `{record.id}` templates count: they interpolate to one scalar - * id, and #3810 already refuses the node outright when such a template erases - * to nothing, so a "scalar" that vanished never reaches the write. + * A CEL value envelope at `id` (`{ dialect: 'cel', source: 'record.id' }`) + * counts: the executor evaluates it to one scalar id before the query, and + * #3810 refuses the node outright when it evaluates to nothing, so a "scalar" + * that vanished never reaches the write (#19939: a filter operand is a value + * slot, the single-brace `{record.id}` it replaced is refused). */ function namesOneRowById(filter: unknown): boolean { if (!filter || typeof filter !== 'object') return false; const keys = Object.keys(filter as Record); if (keys.length !== 1 || keys[0] !== 'id') return false; const id = (filter as { id: unknown }).id; - return typeof id === 'string' || typeof id === 'number'; + return typeof id === 'string' || typeof id === 'number' || isExpressionEnvelopeShaped(id); } function isNonEmptyPredicate(filter: unknown): boolean { diff --git a/examples/app-todo/src/flows/task.flow.ts b/examples/app-todo/src/flows/task.flow.ts index 8c99aacf397..25c82e0223c 100644 --- a/examples/app-todo/src/flows/task.flow.ts +++ b/examples/app-todo/src/flows/task.flow.ts @@ -55,6 +55,9 @@ export const TaskReminderFlow: Flow = { // `limit > 1` is the declared way to make this a LIST read (`find`, not // `findOne`) — the undeclared `getAll` that sat here was never read, so // this sweep silently fetched a single task (#4277 rejects the key now). + // `{tomorrow}` is a query placeholder, the query engine's own vocabulary: + // a filter operand hands it on untouched. A value the FLOW computes is a + // CEL value envelope instead (`get_task` below). config: { objectName: 'todo_task', filter: { due_date: '{tomorrow}', status: { $ne: 'completed' } }, outputVariable: 'tasksToRemind', limit: 200 }, }, { @@ -203,7 +206,7 @@ export const OverdueEscalationFlow: Flow = { id: 'update_priority', type: 'update_record', label: 'Escalate Priority', config: { objectName: 'todo_task', - filter: { id: '{currentTask.id}' }, + filter: { id: { dialect: 'cel', source: 'currentTask.id' } }, fields: { priority: 'urgent', tags: ['important', 'follow_up'] }, }, }, @@ -322,7 +325,7 @@ export const TaskCompletionFlow: Flow = { }, { id: 'get_task', type: 'get_record', label: 'Get Completed Task', - config: { objectName: 'todo_task', filter: { id: '{record.id}' }, outputVariable: 'completedTask' }, + config: { objectName: 'todo_task', filter: { id: { dialect: 'cel', source: 'record.id' } }, outputVariable: 'completedTask' }, }, // A plain exclusive gateway — the branching is on the OUT-EDGES (e3/e4 // carry the predicate and its negation). It used to also set diff --git a/examples/app-todo/test/loop-body-iteration.test.ts b/examples/app-todo/test/loop-body-iteration.test.ts index 968d4d019ea..2556232fdd9 100644 --- a/examples/app-todo/test/loop-body-iteration.test.ts +++ b/examples/app-todo/test/loop-body-iteration.test.ts @@ -235,10 +235,13 @@ describe('#19206 — the loop body runs and binds `currentTask`', () => { const result: any = await automation.execute(legacy.name); expect(result.success).toBe(false); - // The exact refusal the card reports, and the reason it is loud rather than - // catastrophic: an erased filter condition is refused, not widened. - expect(String(result.error)).toContain('update_record: refusing to run'); - expect(String(result.error)).toContain('{currentTask.id}'); + // Loud rather than catastrophic: the filter operand reads `currentTask`, + // which the body-less shape never binds, and the operand envelope fails the + // run before the query, naming it (#19939: a filter operand is a CEL value + // envelope; the single-brace token it replaced erased its condition and the + // node was refused) — the filter is never widened either way. + expect(String(result.error)).toContain('filter.id: value expression failed to evaluate as CEL'); + expect(String(result.error)).toContain('currentTask'); // And the observable the card is about: nothing was notified, because the // run never reached the node that notifies. diff --git a/packages/lint/src/authoring-rules.ts b/packages/lint/src/authoring-rules.ts index 0486b052387..5acaa800652 100644 --- a/packages/lint/src/authoring-rules.ts +++ b/packages/lint/src/authoring-rules.ts @@ -107,7 +107,6 @@ import { validateDatasetMeasureAggregates } from './validate-dataset-measure-agg import { validateDashboardActionRefs } from './validate-dashboard-action-refs.js'; import { validateDashboardWidgetOptions } from './validate-dashboard-widget-options.js'; import { validateFilterTokens } from './validate-filter-tokens.js'; -import { validateFlowFilterTokens } from './validate-flow-filter-tokens.js'; import { validatePresetComparands } from './validate-preset-comparands.js'; import { validateEmptyCombinators } from './validate-empty-combinators.js'; import { validateReferenceIntegrity } from './reference-integrity-suite.js'; @@ -926,26 +925,6 @@ export const AUTHORING_RULES: readonly AuthoringRule[] = [ surfaceReason: RUNTIME_NEEDS_FULL_SNAPSHOT, run: (stack) => validateFilterTokens(stack), }, - // #16096 — the FLOW half of the same question, and a different answer, - // because a flow node's `config.filter` is evaluated by the automation - // template evaluator before ObjectQL ever sees it. Reports only the class - // NEITHER dialect resolves: a call to a name outside the flow template - // dialect's closed function table, where `resolveToken` raises a guard - // refusal and the node cannot run. The open arm (bare/dotted identifiers - // addressing the run's VariableMap) is deliberately left silent — judging it - // against the ObjectQL vocabulary reports 7 findings on this repo's own - // examples, all 7 false positives. Reads `flows` alone, so the per-write - // snapshot carries everything it needs. - { - name: 'validateFlowFilterTokens', - tier: 'gating', - input: 'parsed', - commands: ALL, - source: 'packages/lint/src/validate-flow-filter-tokens.ts', - surfaces: CLI_AND_RUNTIME, - runtimeTypes: ['flow'], - run: (stack) => validateFlowFilterTokens(stack), - }, // #8793 (the ruled C half of #8690) — a declared dashboard date-range preset // name (`last_30_days`, …) authored as a bare ORDERING comparand resolves in // no layer: the engine refuses it on a declared temporal field at query time diff --git a/packages/lint/src/flow-template-grammar.test.ts b/packages/lint/src/flow-template-grammar.test.ts index 340bc347ffc..fe7c348a1d1 100644 --- a/packages/lint/src/flow-template-grammar.test.ts +++ b/packages/lint/src/flow-template-grammar.test.ts @@ -83,9 +83,15 @@ describe('the mirrored grammar still matches the evaluator that owns it', () => expect(source).toContain('throw unknownFunctionError(match, trimmed)'); }); - it('the filter position still hands an unresolved KNOWN filter token to the engine', () => { - // The layer-one/layer-two split the rule is built on. - expect(source).toContain('isKnownFilterToken'); + // [#19939, stage S6] RE-JUDGED IN PLACE: this pinned the evaluator's filter + // hand-off (`interpolateFilter` asking `isKnownFilterToken`), the split the + // retired `flow-filter-token-unknown` was built on. A filter operand reads + // no flow token since S6 — the spec's one classifier tells a query + // placeholder from a refused token — so the evaluator no longer has a + // filter position, and this mirror is read by `lint-flow-patterns.ts` alone. + it('the evaluator no longer reads a filter position — no hand-off to the query placeholders', () => { + expect(source).not.toContain('isKnownFilterToken'); + expect(source).not.toContain('export function interpolateFilter'); }); }); diff --git a/packages/lint/src/flow-template-grammar.ts b/packages/lint/src/flow-template-grammar.ts index 1d5470671af..031bae79a66 100644 --- a/packages/lint/src/flow-template-grammar.ts +++ b/packages/lint/src/flow-template-grammar.ts @@ -3,30 +3,27 @@ /** * @module flow-template-grammar * - * **Which `{…}` dialect owns a whole-string token in a FLOW node's filter, and - * which spellings neither dialect can resolve** (#16096). + * **How the 17.x flow template evaluator dispatches one whole-string `{…}` + * token** — `resolveToken` in `@objectstack/service-automation`'s + * `src/builtin/template.ts`, mirrored for the lint, which cannot import a + * runtime. * - * A filter value position inside a flow node is the one place two `{…}` - * vocabularies meet, and `interpolateFilter` - * (`@objectstack/service-automation`, `src/builtin/template.ts`, #3810) is the - * function that arbitrates them. Its own header states the split: + * It was written (#16096) to say which of two vocabularies owned a token in a + * flow CRUD node's `filter`: the flow template dialect, resolved before the + * query, or the query placeholders (`{current_user_id}`, `{today}`) handed on + * to ObjectQL. That position reads no flow token since #19939 (stage S6): a + * filter operand is a CEL value envelope, a query placeholder or a literal, + * told apart by the spec's one classifier (`classifyFlowFilterOperandString`, + * `@objectstack/spec/automation`), and a flow token there is refused by the + * value-slot judge at every door — so `flow-filter-token-unknown`, the rule + * that read this grammar at that position, retired with it. What still reads + * {@link classifyFlowTemplateToken} is `lint-flow-patterns.ts`, to tell the + * run user (`$User.*`) from a variable when it prescribes a bare `$` + * reference's envelope. The grammar goes with the single-brace resolver + * (ADR-0032 Sequencing 6). * - * > A whole-string token that (a) no flow variable resolves and (b) IS a - * > recognised filter placeholder is passed through **verbatim** for the engine - * > to expand. That is a transfer of ownership, not a lenient fallback. - * - * So a token in this position falls in exactly one of three classes: - * - * | class | resolved by | example | judged here? | - * |---|---|---|---| - * | flow template dialect | the automation template evaluator, BEFORE the query | `{TODAY() - 45}`, `{record.id}`, `{$User.Id}`, `{round(x)}` | ⛔ no | - * | filter placeholder dialect | ObjectQL, after hand-off (`isKnownFilterToken`) | `{current_user_id}`, `{30_days_ago}` | ⛔ no — `filter-token-unknown` owns it | - * | **neither** | nothing — the run fails or the condition collapses | `{TOMORROW()}`, `{ROUND(x)}` | ✅ the third class, and only it | - * - * ## Why only the CALL-POSITION half of the third class is decidable here - * - * The flow dialect's vocabulary is closed in three of its four arms and OPEN in - * the fourth: + * The dialect's vocabulary is closed in three of its four arms and OPEN in the + * fourth: * * - `NOW()` / `TODAY()` with an optional `± N` day offset — closed, two names. * - `$User.` — closed prefix. @@ -37,17 +34,7 @@ * **OPEN**: it addresses the run's `VariableMap`, which holds the flow's * declared variables, every node's `outputVariable`, and — via * `seedRunVariables` — the trigger record's own fields flattened to top - * level. None of that is decidable from authored metadata alone, and a flow - * bound to an object another package defines cannot be resolved here at all. - * - * That asymmetry is the whole reason this module reports the call-position arm - * and nothing else. Measured on this repo's own examples, judging the OPEN arm - * against the ObjectQL vocabulary — the shape #16096 calls "the obvious fix" — - * reports **7 findings at `error`, all 7 false positives** (`{recordId}` ×3, - * `{record.id}` ×3, `{currentTask.id}` ×1, across app-todo / app-crm / - * app-showcase). Every one is a legitimate flow variable that resolves at run - * time. A reference set that reds working sweeps is worse than the silence - * #16096 reports, so the open arm stays unjudged and says so. + * level. None of that is decidable from authored metadata alone. * * ## Dispatch ORDER is load-bearing, not incidental * @@ -116,26 +103,24 @@ export type FlowTemplateTokenVerdict = /** `{$User.Id}` — the evaluator resolves it from the run context. */ | { kind: 'user-context' } /** - * `{recordId}` / `{record.id}` — a `VariableMap` lookup, and the position - * from which an unresolved name is handed to the filter dialect. OPEN: not - * decidable from authored metadata, so never a finding. + * `{recordId}` / `{record.id}` — a `VariableMap` lookup. OPEN: not + * decidable from authored metadata. */ | { kind: 'variable-path'; head: string } /** * A call to a name in NEITHER table. `resolveToken` throws * `FlowExpressionFunctionError` here (a guard refusal — a `fault` edge must - * not swallow it), so the node cannot run. THIS is the finding. + * not swallow it). */ | { kind: 'unknown-function'; name: string } /** * Anything else — junk shapes (`{30 days ago}`) and arithmetic over names - * this module cannot resolve. `resolveToken` answers `undefined` and the - * CRUD collapse guard (#3810) reports it at run time. Open, not judged. + * this module cannot resolve. `resolveToken` answers `undefined`. */ | { kind: 'unresolvable-shape' }; /** - * Classify the INSIDE of one whole-string `{…}` filter token — `inner` is the + * Classify the INSIDE of one whole-string `{…}` token — `inner` is the * text between the braces, exactly as authored. * * Mirrors `resolveToken`'s dispatch order (see the module header). Holds no diff --git a/packages/lint/src/index.ts b/packages/lint/src/index.ts index 4e16b56e6b2..a9154f5def1 100644 --- a/packages/lint/src/index.ts +++ b/packages/lint/src/index.ts @@ -399,8 +399,6 @@ export { validateDashboardWidgetOptions } from './validate-dashboard-widget-opti export type { DashboardWidgetOptionFinding } from './validate-dashboard-widget-options.js'; export { validateFilterTokens, FILTER_TOKEN_UNKNOWN } from './validate-filter-tokens.js'; -export { validateFlowFilterTokens, FLOW_FILTER_TOKEN_UNKNOWN } from './validate-flow-filter-tokens.js'; -export type { FlowFilterTokenFinding } from './validate-flow-filter-tokens.js'; export { classifyFlowTemplateToken, FLOW_TEMPLATE_DATE_FUNCTIONS, FLOW_TEMPLATE_VALUE_FUNCTIONS } from './flow-template-grammar.js'; export type { FlowTemplateTokenVerdict } from './flow-template-grammar.js'; export type { FilterTokenFinding, FilterTokenSeverity } from './validate-filter-tokens.js'; diff --git a/packages/lint/src/lint-flow-patterns.test.ts b/packages/lint/src/lint-flow-patterns.test.ts index a97439c6a14..097b6ec4c55 100644 --- a/packages/lint/src/lint-flow-patterns.test.ts +++ b/packages/lint/src/lint-flow-patterns.test.ts @@ -173,7 +173,7 @@ describe('lintFlowPatterns — date-equality in query filter (#1874)', () => { expect(lintFlowPatterns(filterFlow({ status: 'open', due_date: { $lt: CEL('today()') } }))).toHaveLength(0); }); it('equality against a non-time value (status, interpolated id)', () => { - expect(lintFlowPatterns(filterFlow({ status: 'active', id: '{record.id}' }))).toHaveLength(0); + expect(lintFlowPatterns(filterFlow({ status: 'active', id: CEL('record.id') }))).toHaveLength(0); expect(lintFlowPatterns(filterFlow({ amount: { $eq: CEL('record.threshold') } }))).toHaveLength(0); }); }); @@ -1781,14 +1781,18 @@ describe('lintFlowPatterns — unbounded bulk write (#5482)', () => { ).toHaveLength(0); }); - it('a filter whose only condition is a TEMPLATE — that is #3810\'s fact, at run time', () => { - // `{record.ownr}` (a typo) interpolates to nothing and the run-time guard - // REFUSES the node. At authoring time the condition is written, so warning - // "nothing bounds this" here would be false — and would put two diagnostics - // on one defect, one of them wrong about what the author did. + // [#19939, stage S6] RE-JUDGED IN PLACE: the operand was a `{record.ownr}` + // token, refused at every door since S6; the run-time fact is the same for + // an operand envelope that evaluates to nothing. + it('a filter whose only operand is COMPUTED — that is #3810\'s fact, at run time', () => { + // An operand envelope that evaluates to nothing (a record with no owner) + // and the run-time guard REFUSES the node. At authoring time the + // condition is written, so warning "nothing bounds this" here would be + // false — and would put two diagnostics on one defect, one of them wrong + // about what the author did. expect( lintFlowPatterns(purgeFlow('delete_record', { - objectName: 'lead', filter: { owner: '{record.ownr}' }, multi: true, + objectName: 'lead', filter: { owner: CEL('record.owner') }, multi: true, })), ).toHaveLength(0); }); @@ -2000,7 +2004,7 @@ describe('lintFlowPatterns — unbounded bulk write (#5482)', () => { expect(lintFlowPatterns(loopBodyFlow({ nodes: [{ id: 'sweep', type: 'delete_record', - config: { objectName: 'campaign_member', filter: { lead_id: '{lead.id}' }, multi: true }, + config: { objectName: 'campaign_member', filter: { lead_id: CEL('lead.id') }, multi: true }, }], edges: [], // Scoped to this rule: a bounded write is still an uncontained one, so @@ -2079,10 +2083,10 @@ describe('per-iteration containment (#13681 / #14394)', () => { it('flags every fallible builtin in the body, once each', () => { const fallible = [ - { id: 'n1', type: 'get_record', label: 'Get', config: { objectName: 'case', filter: { id: '{currentCase.id}' } } }, + { id: 'n1', type: 'get_record', label: 'Get', config: { objectName: 'case', filter: { id: CEL('currentCase.id') } } }, { id: 'n2', type: 'create_record', label: 'Create', config: { objectName: 'task' } }, - { id: 'n3', type: 'update_record', label: 'Update', config: { objectName: 'case', fields: { seen: true }, filter: { id: '{currentCase.id}' } } }, - { id: 'n4', type: 'delete_record', label: 'Delete', config: { objectName: 'task', filter: { case_id: '{currentCase.id}' } } }, + { id: 'n3', type: 'update_record', label: 'Update', config: { objectName: 'case', fields: { seen: true }, filter: { id: CEL('currentCase.id') } } }, + { id: 'n4', type: 'delete_record', label: 'Delete', config: { objectName: 'task', filter: { case_id: CEL('currentCase.id') } } }, { id: 'n5', type: 'http', label: 'Call', config: { url: 'https://example.test' } }, { id: 'n6', type: 'notify', label: 'Notify', config: { title: 'x' } }, { id: 'n7', type: 'connector_action', label: 'Connector', config: { connectorId: 'c', action: 'a' } }, @@ -2686,7 +2690,7 @@ describe('#16405 — an `http` node payload is not a region, and both #1315 rule }); it('reads a `$` root the engine binds as that variable (`$error.message`)', () => { - const [f] = bareDollar(valueFlow('update_record', { objectName: 'task', filter: { id: '{record.id}' }, fields: { note: '$error.message' } })); + const [f] = bareDollar(valueFlow('update_record', { objectName: 'task', filter: { id: CEL('source.id') }, fields: { note: '$error.message' } })); expect(f!.message).toContain('in the update_record field value'); expect(f!.hint).toContain(judgeSpelling('{$error.message}')); expect(prescribedSources(f!.hint!)).toEqual(['vars["$error"].message']); @@ -2696,7 +2700,9 @@ describe('#16405 — an `http` node payload is not a region, and both #1315 rule ['$User.Id', 'create_record', (v: unknown) => ({ objectName: 'task', fields: { v } })], ['$source.id', 'create_record', (v: unknown) => ({ objectName: 'task', fields: { v } })], ['$User.Email', 'create_record', (v: unknown) => ({ objectName: 'task', fields: { v } })], - ['$error.message', 'update_record', (v: unknown) => ({ objectName: 'task', filter: { id: '{record.id}' }, fields: { v } })], + // [#19939, stage S6] The filter operand is an envelope over a bound + // variable: a `{record.id}` token there is refused since S6. + ['$error.message', 'update_record', (v: unknown) => ({ objectName: 'task', filter: { id: CEL('source.id') }, fields: { v } })], ['$User.Id', 'assignment', (v: unknown) => ({ assignments: { v } })], // [#19939] The maps a node hands to a callee joined the value slots: // the hint flips there by asking the judge, with no line of its own. @@ -2794,14 +2800,23 @@ describe('#16405 — an `http` node payload is not a region, and both #1315 rule expect(refusalsOf(valueFlow('create_record', { objectName: 'task', fields: { who: CEL(guard) } }))).toEqual(NONE); }); - it('control: outside the value and text slots the single brace still resolves, and the hint still names it', () => { - // [#19939] A CRUD `filter` value — the position S6 has not reached; an - // `http` body is a value slot since S5. + // [#19939, stage S6] RE-JUDGED IN PLACE: the control's position was a + // CRUD `filter` value — "the position S6 has not reached". Each filter + // operand is a value slot since S6, so the hint there names the operand's + // envelope and the filter operand's remedy; the single-brace arm is kept + // for a position no slot judges (a plugin node's own config). + it('control: a filter operand names its envelope now; outside every slot the hint keeps its single-brace arm', () => { const filtered = (owner: string) => valueFlow('get_record', { objectName: 'task', filter: { owner } }); const [f] = bareDollar(filtered('$source.id')); - expect(f!.hint).toContain('Wrap it and bind a variable: `{source.id}`'); - expect(flowNodeValueTemplateRefusals('get_record', { objectName: 'task', filter: { owner: '{source.id}' } })).toEqual([]); - // …and the http body names its envelope now. + expect(f!.message).toContain('in the get_record filter operand'); + expect(f!.hint).toContain('A value slot reads a CEL value envelope'); + expect(f!.hint).toContain('an operand envelope is evaluated before the query'); + expect(prescribedSources(f!.hint!)).toEqual(['source.id']); + expect(refusalsOf(valueFlow('get_record', { objectName: 'task', filter: { owner: CEL('source.id') } }))).toEqual(NONE); + expect(flowNodeValueTemplateRefusals('get_record', { objectName: 'task', filter: { owner: '{source.id}' } })).toHaveLength(1); + const [plugin] = bareDollar(valueFlow('acme_custom_step', { note: '$source.id' })); + expect(plugin!.hint).toContain('Wrap it and bind a variable: `{source.id}`'); + // …and the http body names its envelope. const [body] = lintFlowPatterns(httpFlow({ ticket: '$source.id' })).filter((x) => x.rule === FLOW_BARE_DOLLAR_REF); expect(body!.hint).toContain('A value slot reads a CEL value envelope'); }); @@ -2902,10 +2917,15 @@ describe('#16405 — an `http` node payload is not a region, and both #1315 rule expect(refusalsOf(valueFlow('screen', configOf(CEL("'Re: ' + record.title"))))).toEqual(NONE); }); - it('control: outside the value and text slots the single brace still resolves, and the double-brace hint still names it', () => { - // [#19939] A CRUD `filter` value — the position S6 has not reached. + // [#19939, stage S6] RE-JUDGED IN PLACE, as the bare-`$` control above: + // a CRUD `filter` operand is a value slot since S6. + it('control: a filter operand names its envelope now; outside every slot the double-brace hint keeps its single-brace arm', () => { const [f] = doubleBrace(valueFlow('get_record', { objectName: 'task', filter: { title: '{{ record.title }}' } })); - expect(f!.hint).toContain('Use `{var}` (e.g. `{record.title}`)'); + expect(f!.message).toContain('the get_record filter operand reads a CEL value envelope, not a template'); + expect(f!.hint).not.toContain('Use `{var}`'); + expect(prescribedSources(f!.hint!)).toEqual(['record.title']); + const [plugin] = doubleBrace(valueFlow('acme_custom_step', { note: '{{ record.title }}' })); + expect(plugin!.hint).toContain('Use `{var}` (e.g. `{record.title}`)'); // …and the http body, a value slot since S5, names its envelope. const [body] = lintFlowPatterns(httpFlow({ title: '{{ record.title }}' })).filter((x) => x.rule === FLOW_DOUBLE_BRACE_INTERP); expect(body!.hint).not.toContain('Use `{var}`'); diff --git a/packages/lint/src/lint-flow-patterns.ts b/packages/lint/src/lint-flow-patterns.ts index 037fc419ef3..64944afc2f0 100644 --- a/packages/lint/src/lint-flow-patterns.ts +++ b/packages/lint/src/lint-flow-patterns.ts @@ -1344,9 +1344,9 @@ function scanBranchRouting( * Everything else is left alone, deliberately: * * - **any node carrying a real predicate** (verdict `'clause'`) — including an - * authored `{token}` that will interpolate to nothing. That is the #3810 - * guard's fact, judged at run time against the interpolation result, and this - * rule must not pre-empt it: at authoring time the condition IS written. + * operand envelope that will evaluate to nothing. That is the #3810 guard's + * fact, judged at run time against the evaluated operand, and this rule must + * not pre-empt it: at authoring time the condition IS written. * - **a non-object `filter`** (string, array, number). `DeleteRecordConfigSchema` * /`UpdateRecordConfigSchema` type it `z.record(z.string(), z.unknown())`, so * the node is refused BY NAME at execute time (`parseNodeConfig`). Warning @@ -1412,7 +1412,8 @@ function previewFilter(filter: unknown): string { * ## Not a second copy of the #3810 guard * * `crud-nodes.ts` refuses a node at run time when a condition the author WROTE - * interpolated to nothing (`{record.ownr}` — a typo — leaving `{}`), and it is + * evaluated to nothing (an operand envelope answering `null`, which would + * leave `{}` or match every empty field), and it is * deliberately keyed on "a condition the author wrote is gone", not on "the * filter is empty": losing one of two conditions still widens the blast radius, * and an intentionally empty filter erases nothing, so `crud-filter-guard.test.ts` @@ -1425,8 +1426,9 @@ function previewFilter(filter: unknown): string { * | a written condition vanished | #3810 filter guard | run time | refuse | * | no condition was ever written | this rule | authoring | warn | * - * A node with `filter: { owner: '{record.ownr}' }` is silent here (a condition - * IS written) and refused there. A node with no `filter` at all is warned about + * A node with `filter: { owner: { dialect: 'cel', source: 'record.owner' } }` is + * silent here (a condition IS written) and refused there when the record has + * no owner. A node with no `filter` at all is warned about * here and — correctly — allowed there. */ function scanUnboundedBulkWrites( @@ -1464,7 +1466,7 @@ function scanUnboundedBulkWrites( `the engine admits a whole-object write declared that way, so this is a warning, not a gate, and the ` + `run-time path stays open. Distinct from the run-time erased-condition guard, which REFUSES this node ` + `at run time when ` + - `a condition you WROTE interpolated to nothing — that guard is keyed on "a written condition is gone" ` + + `a condition you WROTE evaluated to nothing — that guard is keyed on "a written condition is gone" ` + `and deliberately not on "the filter is empty", which is the fact this rule judges at authoring ` + `time.`, // Warning, not `error`: see the severity policy at the top of this file. diff --git a/packages/lint/src/rule-explanations.ts b/packages/lint/src/rule-explanations.ts index e8b47936983..7a4bc32381a 100644 --- a/packages/lint/src/rule-explanations.ts +++ b/packages/lint/src/rule-explanations.ts @@ -3463,35 +3463,6 @@ const AI_SKILL_TOOL_UNRESOLVED_EXPLANATION: RuleExplanation = { ], }; -// ── Flow filter tokens (`validate-flow-filter-tokens.ts`) ────────────────── - -const FLOW_FILTER_TOKEN_UNKNOWN_EXPLANATION: RuleExplanation = { - rule: 'flow-filter-token-unknown', - covers: 'the two vocabularies a flow filter token resolves in', - paragraphs: [ - // The function lists are held to `FLOW_TEMPLATE_DATE_FUNCTIONS` and - // `FLOW_TEMPLATE_VALUE_FUNCTIONS` by the rule's test. - 'A flow node\'s filter value is resolved in two layers. The automation template evaluator reads ' + - 'it first, in the flow template dialect: `{NOW()}` and `{TODAY()}` with an optional ± N day ' + - 'offset, `{$User.Id}`, `{record.}`, and the value functions round, floor, ceil, abs, ' + - 'min and max. Only a token that evaluator does not resolve is handed on to ObjectQL ' + - '(`interpolateFilter`), whose filter placeholders (`{current_user_id}`, `{today}`, ' + - '`{30_days_ago}` and the rest) resolve it there.', - 'A token in call position that is a function in neither vocabulary never reaches the query: ' + - '`resolveToken` throws `FlowExpressionFunctionError`, a guard refusal a `fault` edge may not ' + - 'swallow, so the node cannot run at all. That is why this is an error. An unknown token in a ' + - 'presentation filter fails differently, reaching the data engine as a literal so the surface ' + - 'renders empty, and `filter-token-unknown` reports that one.', - 'Judged: the string values inside the authored filters of `flows`, and only the call-position ' + - 'arm, whose reference set is closed. Bare and dotted identifiers (`{recordId}`, ' + - '`{record.id}`) address the run\'s variable map, which is open, so they are never judged here, ' + - 'and a junk shape is reported by the CRUD guard at run time. This is its own id rather than ' + - '`flows` added to `filter-token-unknown`\'s walk, because the two positions share neither the ' + - 'vocabulary nor the consequence, and a machine consumer keyed on the id cannot see which ' + - 'position fired.', - ], -}; - // ── Managed-object API methods (`validate-managed-api-methods.ts`) ───────── const MANAGED_API_METHOD_UNAFFORDABLE_EXPLANATION: RuleExplanation = { @@ -4025,7 +3996,6 @@ export const RULE_EXPLANATIONS: Readonly> = Obje [FLOW_CREDENTIAL_LITERAL_EXPLANATION.rule]: FLOW_CREDENTIAL_LITERAL_EXPLANATION, [AI_SKILL_SURFACE_MISMATCH_EXPLANATION.rule]: AI_SKILL_SURFACE_MISMATCH_EXPLANATION, [AI_SKILL_TOOL_UNRESOLVED_EXPLANATION.rule]: AI_SKILL_TOOL_UNRESOLVED_EXPLANATION, - [FLOW_FILTER_TOKEN_UNKNOWN_EXPLANATION.rule]: FLOW_FILTER_TOKEN_UNKNOWN_EXPLANATION, [MANAGED_API_METHOD_UNAFFORDABLE_EXPLANATION.rule]: MANAGED_API_METHOD_UNAFFORDABLE_EXPLANATION, [ORG_AXIS_PERMISSION_INHERITANCE_EXPLANATION.rule]: ORG_AXIS_PERMISSION_INHERITANCE_EXPLANATION, [ORG_AXIS_CROSS_ORG_BU_GRANT_EXPLANATION.rule]: ORG_AXIS_CROSS_ORG_BU_GRANT_EXPLANATION, diff --git a/packages/lint/src/validate-expressions.fields-value-slot.test.ts b/packages/lint/src/validate-expressions.fields-value-slot.test.ts index a1a3c96dc3a..8b75486052f 100644 --- a/packages/lint/src/validate-expressions.fields-value-slot.test.ts +++ b/packages/lint/src/validate-expressions.fields-value-slot.test.ts @@ -51,7 +51,7 @@ function stackWith(nodeType: NodeType, config: Record) { name: 'price_quote', label: 'Price Quote', type: 'autolaunched', - variables: [{ name: 'price', type: 'number', isInput: true }], + variables: [{ name: 'price', type: 'number', isInput: true }, { name: 'quoteId', type: 'text', isInput: true }], nodes: [ { id: 'start', type: 'start', label: 'Start' }, { id: 'w', type: nodeType, label: 'Write', config }, @@ -80,7 +80,7 @@ function validate(nodeType: NodeType, config: Record) { const crud = (nodeType: 'create_record' | 'update_record', fields: Record) => nodeType === 'create_record' ? { objectName: 'quote', fields } - : { objectName: 'quote', filter: { id: '{quoteId}' }, fields }; + : { objectName: 'quote', filter: { id: { dialect: 'cel', source: 'quoteId' } }, fields }; const NODE_TYPES = ['create_record', 'update_record'] as const; @@ -172,8 +172,17 @@ describe('the retired template dialect — a `{…}` token in a value slot is a expect(validate('assignment', { assignments: { v: value } })).toEqual([]); }); - it('says nothing outside a value slot — `update_record.filter` is a match map, not a value', () => { - expect(validate('update_record', { objectName: 'quote', filter: { total: '{round(price)}' }, fields: { subject: 'x' } })) - .toEqual([]); + // [#19939, stage S6] RE-JUDGED IN PLACE: this pinned `update_record.filter` + // as no value slot, its token unjudged. Each filter operand is a value slot + // since S6, so the token is refused at the operand with its envelope — and a + // query placeholder there stays the query engine's, unjudged. + it('a filter operand is a value slot too — a token is refused there; a query placeholder is not', () => { + const findings = validate('update_record', { + objectName: 'quote', filter: { total: '{round(price)}', due: { $lt: '{today}' } }, fields: { subject: 'x' }, + }); + expect(findings.map((f) => [f.rule, f.severity])).toEqual([[EXPRESSION_INVALID, 'error']]); + expect(findings[0]!.where).toContain('update_record filter operand at config.filter.total'); + expect(findings[0]!.message.startsWith(VALUE_SLOT_TEMPLATE_REFUSAL)).toBe(true); + expect(findings[0]!.message).toContain("{ dialect: 'cel', source: 'round(price)' }"); }); }); 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 a43095adce5..a7355ad2416 100644 --- a/packages/lint/src/validate-expressions.flow-cel-root.test.ts +++ b/packages/lint/src/validate-expressions.flow-cel-root.test.ts @@ -140,8 +140,11 @@ describe('every flow CEL site the door visits', () => { "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') } } }] }), + ['an update_record field envelope', flowStack({ middle: [{ id: 'up', type: 'update_record', config: { objectName: 'acct', filter: { id: 'acc_1' }, fields: { status: env('user.id') } } }] }), "flow 'f' · node 'up' (update_record) update_record field value at config.fields.status"], + // [#19939, stage S6] A filter operand is a value slot: its envelope is a CEL site the door visits. + ['a filter operand envelope', flowStack({ middle: [{ id: 'rm', type: 'delete_record', config: { objectName: 'acct', filter: { owner: { $eq: env('user.id') } } } }] }), + "flow 'f' · node 'rm' (delete_record) delete_record filter operand at config.filter.owner.$eq"], ])('%s', (_site, stack, where) => { const found = rootFindings(stack); expect(found.map((i) => i.where)).toEqual([where]); @@ -169,8 +172,8 @@ describe('every flow CEL site the door visits', () => { 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 `outputVariable`', { edgeCondition: 'acc.status == "x"', middle: [{ id: 'read', type: 'get_record', config: { objectName: 'acct', filter: { id: 'acc_1' }, 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: 'acc_1' } } }] }], ['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: { dialect: 'cel', source: '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: { dialect: 'cel', source: 'rows' }, body: { nodes: [], edges: [] } } }] }], diff --git a/packages/lint/src/validate-expressions.test.ts b/packages/lint/src/validate-expressions.test.ts index 806c046ac69..3c6d31d1696 100644 --- a/packages/lint/src/validate-expressions.test.ts +++ b/packages/lint/src/validate-expressions.test.ts @@ -5109,6 +5109,59 @@ describe('the notify literals are refused with the literal remedy (#19939)', () * replaces it — except `signingSecret`, whose credential store may hold the * secret the run signs with, so its refusal is the executor's parse. */ +/** + * [#19939, stage S6] Each operand of a CRUD node's `filter` is a value slot + * (Fork R17 → A): a CEL value envelope evaluated before the query, a query + * placeholder the query engine resolves, or a literal. `objectstack validate` + * refuses a flow `{…}` token at every operand position with its envelope, and + * judges an envelope operand as CEL; a placeholder passes untouched. + */ +describe('the CRUD filter operands are value slots; the query placeholders stay (#19939, S6)', () => { + const stackWith = (type: string, filter: unknown) => ({ + flows: [{ + name: 'f', + variables: [{ name: 'owner_id', type: 'text', isInput: true }], + nodes: [ + { id: 'start', type: 'start' }, + { id: 'n', type, config: { objectName: 'deal', filter, ...(type === 'update_record' ? { fields: { stage: 'x' } } : {}) } }, + ], + edges: [], + }], + }); + const errorsOf = (stack: unknown) => + validateStackExpressions(stack as never).filter((i) => (i.severity ?? 'error') === 'error'); + const TYPES = ['get_record', 'update_record', 'delete_record'] as const; + + it.each(TYPES.flatMap((type) => [ + [type, 'a field\'s value', { owner: '{owner_id}' }, 'config.filter.owner'], + [type, 'an operator\'s comparand', { amount: { $gte: '{owner_id}' } }, 'config.filter.amount.$gte'], + [type, 'an `$in` element', { id: { $in: ['d1', '{owner_id}'] } }, 'config.filter.id.$in[1]'], + [type, 'a `$or` member', { $or: [{ owner: '{owner_id}' }] }, 'config.filter.$or[0].owner'], + ] as const))('%s — %s: a flow token is refused at `error`, at the operand, with its envelope', (type, _what, filter, at) => { + const found = errorsOf(stackWith(type, filter)); + expect(found).toHaveLength(1); + expect(found[0]!.where).toContain(`${type} filter operand at ${at}`); + expect(found[0]!.message.startsWith(VALUE_SLOT_TEMPLATE_REFUSAL)).toBe(true); + expect(found[0]!.message).toContain("{ dialect: 'cel', source: 'owner_id' }: an operand envelope is evaluated before the query."); + }); + + it.each(TYPES)('%s: an envelope operand, a literal and a query placeholder are clean', (type) => { + const CEL = (source: string) => ({ dialect: 'cel', source }); + expect(errorsOf(stackWith(type, { + owner: CEL('owner_id'), amount: { $gt: 3 }, id: { $in: [CEL('owner_id'), 'd2'] }, + creator: '{current_user_id}', due: { $lt: '{today}', $gte: '{30_days_ago}' }, status: 'open', + }))).toEqual([]); + }); + + it.each(TYPES)('%s: an envelope operand is judged as CEL — an unbound root and a malformed envelope are refused', (type) => { + const unbound = errorsOf(stackWith(type, { owner: { dialect: 'cel', source: 'nobody.id' } })); + expect(unbound.map((i) => i.where)).toEqual([expect.stringContaining(`${type} filter operand at config.filter.owner`)]); + const malformed = errorsOf(stackWith(type, { owner: { $eq: { dialect: 'cel', source: ' ' } } })); + expect(malformed).toHaveLength(1); + expect(malformed[0]!.where).toContain('config.filter.owner.$eq'); + }); +}); + describe('the http text slots, body and literals are refused with their remedies (#19939)', () => { const stackWith = (config: Record) => ({ flows: [{ name: 'f', nodes: [{ id: 'start', type: 'start' }, { id: 'n', type: 'http', config: { url: 'https://x.test/hook', ...config } }], edges: [] }], diff --git a/packages/lint/src/validate-flow-filter-tokens.test.ts b/packages/lint/src/validate-flow-filter-tokens.test.ts deleted file mode 100644 index f10477d00b7..00000000000 --- a/packages/lint/src/validate-flow-filter-tokens.test.ts +++ /dev/null @@ -1,271 +0,0 @@ -// Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license. - -import { describe, expect, it } from 'vitest'; - -import { - validateFlowFilterTokens as validateFlowFilterTokensUnrecorded, - FLOW_FILTER_TOKEN_UNKNOWN, -} from './validate-flow-filter-tokens.js'; -import { validateFilterTokens } from './validate-filter-tokens.js'; -import { FLOW_TEMPLATE_DATE_FUNCTIONS, FLOW_TEMPLATE_VALUE_FUNCTIONS } from './flow-template-grammar.js'; -import { explainRule } from './rule-explanations.js'; - -// [#22161] Each finding is one verdict sentence; the reasoning it used to -// carry is `os explain flow-filter-token-unknown`. Every direct call below -// records what it fired, and the last cases in this file hold each recorded -// verdict to one line of at most 200 characters — every firing variant this -// suite exercises, not a chosen few. Run the whole file: those cases read what -// the cases above fired. -const fired: Array<{ rule: string; message: string; hint: string }> = []; -const validateFlowFilterTokens: typeof validateFlowFilterTokensUnrecorded = (...args) => { - const findings = validateFlowFilterTokensUnrecorded(...args); - fired.push(...findings); - return findings; -}; - -/** A flow whose `query_stalled` node filters on `value` — #16096's own shape. */ -function flowStack(value: unknown): Record { - return { - flows: [ - { - name: 'opportunity_stagnation', - nodes: [ - { id: 'query_stalled', type: 'query_records', config: { objectName: 'opportunity', filter: { close_date: { $lt: value } } } }, - ], - }, - ], - }; -} - -/** The card's CONTROL: the identical string in a list view's filter. */ -function viewStack(value: unknown): Record { - return { - views: [ - { name: 'account_list', object: 'account', filter: [{ field: 'close_date', operator: 'lt', value }] }, - ], - }; -} - -describe('#16096 — the reach gap, reproduced with the card\'s own control', () => { - it('reports an unresolvable {TOMORROW()} in a flow node config.filter', () => { - const findings = validateFlowFilterTokens(flowStack('{TOMORROW()}')); - expect(findings).toHaveLength(1); - expect(findings[0].rule).toBe(FLOW_FILTER_TOKEN_UNKNOWN); - expect(findings[0].severity).toBe('error'); - expect(findings[0].where).toBe('flow "opportunity_stagnation"'); - expect(findings[0].path).toBe('flows[0].nodes[0].config.filter.close_date.$lt'); - expect(findings[0].message).toContain('TOMORROW'); - // The consequence claim is the flow one, NOT the view one. A message that - // said "renders empty" here would be false: the node refuses to run. - expect(findings[0].message).toContain('cannot run'); - expect(findings[0].message).not.toContain('renders empty'); - }); - - it('CONTROL — the same string in a view filter still fires the ORIGINAL rule, unchanged', () => { - const control = validateFilterTokens(viewStack('{TOMORROW()}')); - expect(control).toHaveLength(1); - expect(control[0].rule).toBe('filter-token-unknown'); - expect(control[0].severity).toBe('error'); - }); - - it('the original rule STILL does not reach flows — this rule did not widen it', () => { - expect(validateFilterTokens(flowStack('{TOMORROW()}'))).toEqual([]); - }); - - it('and the new rule does not reach views — the two surfaces stay disjoint', () => { - expect(validateFlowFilterTokens(viewStack('{TOMORROW()}'))).toEqual([]); - }); -}); - -describe('the NEGATIVE CONTROL — legitimate flow template tokens stay silent', () => { - // Triage's binding instruction: a legitimate `{TODAY() - 45}` must stay - // silent, and it is the easiest thing to regress. It survives only because - // the date-function form is tried BEFORE the call-position scan. - const legitimate = [ - '{TODAY() - 45}', '{TODAY()}', '{TODAY() + 90}', '{NOW()}', '{NOW() - 1}', - '{TODAY()-45}', '{ TODAY() + 7 }', - '{$User.Id}', '{$User.Email}', - '{record.id}', '{recordId}', '{currentTask.id}', '{record.target_channels.0}', - '{round(amount)}', '{max(a, b)}', '{floor(x / 2)}', - '{current_user_id}', '{current_org_id}', '{today}', '{30_days_ago}', '{week_start}', - 'closed', 42, true, null, 'acme {x} deal', '{a}{b}', '{{x}}', - ]; - for (const value of legitimate) { - it(`stays silent on ${JSON.stringify(value)}`, () => { - expect(validateFlowFilterTokens(flowStack(value))).toEqual([]); - }); - } -}); - -describe('the third class — a call to a name NEITHER dialect knows', () => { - const unresolvable: Array<[string, string]> = [ - ['{TOMORROW()}', 'TOMORROW'], - ['{YESTERDAY()}', 'YESTERDAY'], - ['{ROUND(x)}', 'ROUND'], - ['{DATEADD(day, -45)}', 'DATEADD'], - ['{Math.round(x)}', 'Math.round'], - ['{upper(name)}', 'upper'], - ]; - for (const [value, name] of unresolvable) { - it(`reports ${value} naming '${name}'`, () => { - const findings = validateFlowFilterTokens(flowStack(value)); - expect(findings).toHaveLength(1); - expect(findings[0].message).toContain(name); - }); - } - - it("prescribes the whole-token form when TODAY() is called in an expression it cannot carry", () => { - // `{TODAY() - 45 - 10}` misses the `± N` grammar, so the runtime reaches - // the scan and refuses TODAY in call position. The hint must say why. - const findings = validateFlowFilterTokens(flowStack('{TODAY() - 45 - 10}')); - expect(findings).toHaveLength(1); - expect(findings[0].hint).toContain('whole token'); - expect(findings[0].hint).toContain('± N day offset'); - }); - - it("suggests the supported spelling for a case mistake", () => { - const findings = validateFlowFilterTokens(flowStack('{ROUND(x)}')); - expect(findings[0].hint).toContain("Did you mean 'round'?"); - }); - - it('reports the FIRST unknown call, the one the runtime throws on', () => { - const findings = validateFlowFilterTokens(flowStack('{FOO(1) + BAR(2)}')); - expect(findings).toHaveLength(1); - // Assert on the NAMED call, not on the echoed value — the message quotes - // the whole authored string, which contains both names. - expect(findings[0].message).toContain("calls 'FOO'"); - expect(findings[0].message).not.toContain("calls 'BAR'"); - }); -}); - -describe('walk reach', () => { - it('reaches a filter nested inside a container node (try/catch region)', () => { - const stack = { - flows: [{ - name: 'resilient_sync', - nodes: [{ - id: 'guard', type: 'try_catch', - config: { catch: { nodes: [{ id: 'purge', type: 'delete_record', config: { filter: { at: '{TOMORROW()}' } } }] } }, - }], - }], - }; - const findings = validateFlowFilterTokens(stack); - expect(findings).toHaveLength(1); - expect(findings[0].path).toBe('flows[0].nodes[0].config.catch.nodes[0].config.filter.at'); - }); - - it('judges every authored filter shape — triples and rule objects too', () => { - const triple = { flows: [{ name: 'f', nodes: [{ id: 'n', config: { filter: [['close_date', '<', '{TOMORROW()}']] } }] }] }; - const ruleObj = { flows: [{ name: 'f', nodes: [{ id: 'n', config: { filter: [{ field: 'close_date', operator: 'lt', value: '{TOMORROW()}' }] } }] }] }; - expect(validateFlowFilterTokens(triple)).toHaveLength(1); - expect(validateFlowFilterTokens(ruleObj)).toHaveLength(1); - }); - - it('is a strict SUBSET of what a bare `flows` root addition would report', () => { - // Every finding this rule emits is one `classifyFilterToken` also calls - // unknown — the layering the rule is built on, asserted rather than - // assumed. The converse does NOT hold, which is the whole point. - for (const value of ['{TOMORROW()}', '{ROUND(x)}', '{recordId}', '{TODAY() - 45}']) { - const mine = validateFlowFilterTokens(flowStack(value)); - const naive = validateFilterTokens({ ...flowStack(value), views: [{ name: 'v', filter: [{ field: 'f', operator: 'lt', value }] }] }); - if (mine.length > 0) expect(naive.length).toBeGreaterThan(0); - } - }); - - it('tolerates an absent / malformed flows collection', () => { - expect(validateFlowFilterTokens(null)).toEqual([]); - expect(validateFlowFilterTokens({})).toEqual([]); - expect(validateFlowFilterTokens({ flows: 'nope' })).toEqual([]); - }); - - it('guards a cyclic graph', () => { - const cyclic: Record = { filter: {} }; - (cyclic.filter as Record).self = cyclic; - expect(() => validateFlowFilterTokens({ flows: [{ name: 'f', nodes: [cyclic] }] })).not.toThrow(); - }); -}); - -describe('[#22161] one-line verdicts — flow-filter-token-unknown', () => { - it('reads as one sentence', () => { - const findings = validateFlowFilterTokens(flowStack('{TOMORROW()}')); - expect(findings.map((f) => f.message)).toEqual([ - 'Filter value "{TOMORROW()}" calls \'TOMORROW\', which neither the flow template dialect nor the filter ' - + 'placeholders define, so the evaluator raises a guard refusal and this node cannot run', - ]); - }); - - it('`os explain` carries what the verdict no longer says', () => { - const entry = explainRule(FLOW_FILTER_TOKEN_UNKNOWN); - expect(entry, 'no `os explain flow-filter-token-unknown` entry').toBeDefined(); - const text = entry!.paragraphs.join('\n'); - for (const fact of [ - 'two layers', - '`interpolateFilter`', - '`FlowExpressionFunctionError`', - 'a `fault` edge may not swallow', - 'renders empty', - 'call-position', - '`{record.id}`', - 'a machine consumer keyed on the id', - ]) { - expect(text, `explanation names ${fact}`).toContain(fact); - } - }); - - it('the two vocabularies the explanation names are the rule\'s own', () => { - // The explanation module imports nothing, so its function lists are held - // to the grammar's constants here; the placeholders and the template - // forms it names are held by the silent cases above. - const text = explainRule(FLOW_FILTER_TOKEN_UNKNOWN)!.paragraphs.join('\n'); - expect(FLOW_TEMPLATE_DATE_FUNCTIONS).toEqual(['NOW', 'TODAY']); - expect(text).toContain('`{NOW()}` and `{TODAY()}` with an optional ± N day offset'); - const values = [...FLOW_TEMPLATE_VALUE_FUNCTIONS]; - expect(text).toContain(`the value functions ${values.slice(0, -1).join(', ')} and ${values[values.length - 1]}.`); - for (const placeholder of ['{current_user_id}', '{today}', '{30_days_ago}', '{$User.Id}']) { - expect(text, placeholder).toContain(placeholder); - expect(validateFlowFilterTokensUnrecorded(flowStack(placeholder)), placeholder).toEqual([]); - } - }); - - it('every verdict the cases in this file fired is one line of at most 200 characters', () => { - const converted = fired.filter((f) => f.rule === FLOW_FILTER_TOKEN_UNKNOWN); - // The coverage control first: the id fired, on more than one token. - expect(new Set(converted.map((f) => f.message)).size).toBeGreaterThan(1); - for (const f of converted) { - expect(f.message, f.rule).not.toContain('\n'); - expect(f.message.length, `${f.rule}: ${f.message}`).toBeLessThanOrEqual(200); - } - }); -}); - -// [#22161] Slice 14: the `fix:` line. It names the value functions and the -// placeholder alternative in one line (the run-time refusal names the same -// functions); the two vocabularies in full are the explanation's, held to the -// grammar's constants above. -describe('[#22161] one-line fixes — flow-filter-token-unknown', () => { - it('every `fix:` line the cases in this file fired is one line of at most 200 characters', () => { - const converted = fired.filter((f) => f.rule === FLOW_FILTER_TOKEN_UNKNOWN); - // Coverage control: the date-function arm, an arm with no suggestion, and - // the suggestion arm with and without the method-call note. - expect(converted.some((f) => f.hint.includes('is supported only as the whole token'))).toBe(true); - expect(converted.some((f) => f.hint.startsWith('Call a flow value function') && !f.hint.includes('Did you mean'))).toBe(true); - expect(converted.some((f) => f.hint.includes('Did you mean') && !f.hint.includes('Method/namespace'))).toBe(true); - expect(converted.some((f) => f.hint.includes('Method/namespace call syntax is not supported'))).toBe(true); - for (const f of converted) { - expect(f.hint, f.rule).not.toContain('\n'); - expect(f.hint.length, `${f.rule}: ${f.hint}`).toBeLessThanOrEqual(200); - } - }); - - it('the line names the value functions the grammar defines', () => { - const [f] = validateFlowFilterTokens(flowStack('{TOMORROW()}')); - expect(f.hint).toContain(`(${FLOW_TEMPLATE_VALUE_FUNCTIONS.join(', ')})`); - }); - - it('`os explain` carries what the `fix:` line no longer says', () => { - const text = explainRule(FLOW_FILTER_TOKEN_UNKNOWN)!.paragraphs.join('\n'); - for (const fact of ['`{$User.Id}`', '`{record.}`', '`{current_user_id}`', 'handed on to ObjectQL']) { - expect(text, `explanation names ${fact}`).toContain(fact); - } - }); -}); diff --git a/packages/lint/src/validate-flow-filter-tokens.ts b/packages/lint/src/validate-flow-filter-tokens.ts deleted file mode 100644 index 89c60c38378..00000000000 --- a/packages/lint/src/validate-flow-filter-tokens.ts +++ /dev/null @@ -1,178 +0,0 @@ -// Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license. - -/** - * @module validate-flow-filter-tokens - * - * **A `{…}` filter token in a flow node that NEITHER `{…}` dialect can - * resolve** (#16096). - * - * `filter-token-unknown` (`validate-filter-tokens.ts`) walks seven - * presentation collections and deliberately not `flows`, so `{TOMORROW()}` in a - * list view's filter fails the build while the identical string in a flow - * node's `config.filter` is silent. Its siblings already reach flows - * (`validate-empty-combinators.ts`, `validate-preset-comparands.ts`, - * `lint-liveness-properties.ts`), so the gap is a root-list inconsistency, not - * a position the package holds an opinion about. - * - * ## Why this is a SECOND rule id and not `flows` added to that root list - * - * The two positions do not share a vocabulary or a consequence, and - * `filter-token-unknown`'s message is a factual claim about the run — - * *"sent to the data engine as a literal string, matches no record, and the - * surface renders empty"* — that is **false** here in both halves: - * - * - **Vocabulary.** A flow filter is evaluated by the automation template - * evaluator FIRST, and only what that evaluator cannot resolve is handed on - * to ObjectQL (`interpolateFilter`, #3810). Judging a flow filter against the - * ObjectQL vocabulary reports every legitimate `{record.id}` — measured at - * **7 findings, all 7 false positives**, on this repo's own examples. - * `flow-template-grammar.ts` carries that measurement and the boundary. - * - **Consequence.** For the class reported here the run does not query and - * render empty: `resolveToken` throws `FlowExpressionFunctionError`, a guard - * refusal a `fault` edge may not swallow. The node **cannot run at all**. - * - * Widening the published id would make its meaning depend on the position it - * fired in, which a machine consumer keyed on the id cannot see. So the id is - * new and the old rule's declared surface list is untouched. - * - * ## Severity: `error` - * - * The same axis `validate-flow-template-paths.ts` already applies at this exact - * position — an unresolvable token inside a filter-guarded CRUD node is not - * "the output will be blank", it is "this node cannot run", so the build is - * shipping a flow whose runtime is already decided. Gating, not advisory. - * - * ## What it deliberately leaves silent - * - * Everything whose oracle is open: bare and dotted identifiers (`{recordId}`, - * `{record.id}`) address a run-time `VariableMap`, and junk shapes collapse - * into the CRUD guard's own run-time report. Only the CALL-POSITION arm has a - * closed reference set, and only it is reported. `flow-template-grammar.ts` - * states that boundary in full. - */ - -import { nearestName } from '@objectstack/formula'; -import { classifyFilterToken } from '@objectstack/spec/data'; - -import { - classifyFlowTemplateToken, - FLOW_TEMPLATE_DATE_FUNCTIONS, - FLOW_TEMPLATE_VALUE_FUNCTIONS, -} from './flow-template-grammar.js'; -import { walkAuthoredFilters, type FilterSurface } from './filter-walk.js'; - -/** Diagnostic rule id. */ -export const FLOW_FILTER_TOKEN_UNKNOWN = 'flow-filter-token-unknown'; - -export interface FlowFilterTokenFinding { - /** Always `error` — the node throws a guard refusal instead of running. */ - severity: 'error'; - rule: string; - /** Human-readable location, e.g. `flow "opportunity_stagnation"`. */ - where: string; - /** Config path, e.g. `flows[2].nodes[1].config.filter.close_date.$lt`. */ - path: string; - message: string; - hint: string; -} - -/** - * The one collection this rule walks. Declared here rather than shared, for the - * reason `validate-filter-tokens.ts` states about its own list: a shared - * surface constant would let another rule's widening land in a gating rule - * silently. - */ -const FLOW_FILTER_SURFACES: readonly FilterSurface[] = [{ key: 'flows', kind: 'flow' }]; - -/** - * Compose the hint, mirroring `unknownFunctionError`'s own prescription so the - * authoring-time wording and the run-time fault read as one system. - */ -function hintFor(name: string): string { - if (FLOW_TEMPLATE_DATE_FUNCTIONS.includes(name)) { - return `${name}() is supported only as the whole token, with an optional ± N day offset — write {${name}() + 7}.`; - } - const last = name.includes('.') ? name.slice(name.lastIndexOf('.') + 1) : name; - const lower = last.toLowerCase(); - const suggestion = FLOW_TEMPLATE_VALUE_FUNCTIONS.includes(lower) - ? lower - : nearestName(lower, FLOW_TEMPLATE_VALUE_FUNCTIONS); - const didYouMean = suggestion - ? ` Did you mean '${suggestion}'?${name.includes('.') ? ' Method/namespace call syntax is not supported — write the bare form.' : ''}` - : ''; - // [#22161] One remedy line naming the value functions, as the run-time - // refusal does; the two vocabularies in full are the id's `os explain` entry. - return `Call a flow value function (${FLOW_TEMPLATE_VALUE_FUNCTIONS.join(', ')}) or use a filter placeholder.${didYouMean}`; -} - -/** Judge every string inside one authored filter subtree. */ -function walkValues( - node: unknown, - path: string, - where: string, - out: FlowFilterTokenFinding[], - seen: Set, -): void { - if (node === null || node === undefined) return; - - if (typeof node === 'string') { - // Layer one: ask the FILTER dialect first, through the same classifier - // `filter-token-unknown` uses. `null` means the value is not a placeholder - // attempt at all; `context` / `date-macro` mean ObjectQL owns and resolves - // it after the hand-off. Only its `unknown` verdict can possibly be a token - // neither layer knows — which makes every finding here a strict SUBSET of - // what a bare `flows` root addition would report, and `token` is the raw - // text between the braces, exactly as authored. - const filterVerdict = classifyFilterToken(node); - if (filterVerdict?.kind !== 'unknown') return; - // Layer two: ask the FLOW dialect what it does with that same text. - const verdict = classifyFlowTemplateToken(filterVerdict.token); - if (verdict.kind !== 'unknown-function') return; - out.push({ - severity: 'error', - rule: FLOW_FILTER_TOKEN_UNKNOWN, - where, - path, - // [#22161] One verdict sentence; the two vocabularies, why a miss stops - // the node and what is not judged are `os explain flow-filter-token-unknown`. - message: - `Filter value "${node}" calls '${verdict.name}', which neither the flow template dialect ` - + `nor the filter placeholders define, so the evaluator raises a guard refusal and this ` - + `node cannot run`, - hint: hintFor(verdict.name), - }); - return; - } - - if (typeof node !== 'object') return; - // Metadata graphs can be cyclic once normalized; guard the walk. - if (seen.has(node)) return; - seen.add(node); - - if (Array.isArray(node)) { - node.forEach((v, i) => walkValues(v, `${path}[${i}]`, where, out, seen)); - return; - } - - for (const [k, v] of Object.entries(node as Record)) { - walkValues(v, `${path}.${k}`, where, out, seen); - } -} - -/** - * Validate flow-node filter placeholders across a schema-parsed stack. - * - * Pure `(stack) => Finding[]`; no I/O. - */ -export function validateFlowFilterTokens( - stack: Record | undefined | null, -): FlowFilterTokenFinding[] { - if (!stack || typeof stack !== 'object') return []; - const out: FlowFilterTokenFinding[] = []; - - walkAuthoredFilters(stack, FLOW_FILTER_SURFACES, ({ value, path, where }) => { - walkValues(value, path, where, out, new Set()); - }); - - return out; -} diff --git a/packages/lint/src/validate-flow-node-writes.test.ts b/packages/lint/src/validate-flow-node-writes.test.ts index 2e55dca6cf4..c229b233a31 100644 --- a/packages/lint/src/validate-flow-node-writes.test.ts +++ b/packages/lint/src/validate-flow-node-writes.test.ts @@ -68,7 +68,7 @@ function flowWith( id: 'mark', type: 'update_record', label: 'Mark won', - config: { objectName: 'deal', filter: { id: '{recordId}' }, fields, ...nodeConfigOverrides }, + config: { objectName: 'deal', filter: { id: { dialect: 'cel', source: 'recordId' } }, fields, ...nodeConfigOverrides }, ...nodeOverrides, }, ], @@ -568,7 +568,7 @@ const anchorFlow = (fields: unknown) => ({ id: 'mark', type: 'update_record', label: 'Stamp', - config: { objectName: 'wh_order', filter: { id: '{recordId}' }, fields }, + config: { objectName: 'wh_order', filter: { id: { dialect: 'cel', source: 'recordId' } }, fields }, }, ], edges: [], diff --git a/packages/lint/src/validate-flow-template-paths.ts b/packages/lint/src/validate-flow-template-paths.ts index b9030a4d862..7e3855d7bec 100644 --- a/packages/lint/src/validate-flow-template-paths.ts +++ b/packages/lint/src/validate-flow-template-paths.ts @@ -160,8 +160,10 @@ const RELATION_TYPES: ReadonlySet = new Set([ ]); // The CRUD nodes whose `filter` the runtime guards (framework#3810): each calls -// `resolveNodeFilter`, which refuses the node when interpolation erased any -// authored condition. `create_record` is deliberately absent — it writes a +// `resolveNodeFilter`, which refuses the node when an authored operand +// evaluated to nothing. (Since #19939 a single-brace token at a filter operand +// is refused before any run, by the value-slot judge; this rule still reads +// one there until the single-brace reading retires.) `create_record` is deliberately absent — it writes a // payload and has no filter, so an unresolved token there is a blank value on // the new row, not a widened query. const FILTER_GUARDED_NODE_TYPES: ReadonlySet = new Set([ diff --git a/packages/lint/src/validate-readonly-flow-writes.test.ts b/packages/lint/src/validate-readonly-flow-writes.test.ts index a3653aa2da1..df58e8798ff 100644 --- a/packages/lint/src/validate-readonly-flow-writes.test.ts +++ b/packages/lint/src/validate-readonly-flow-writes.test.ts @@ -53,7 +53,7 @@ function flowWith( id: 'stamp', type: 'update_record', label: 'Stamp approval', - config: { objectName: 'crm_opportunity', filter: { id: '{recordId}' }, fields, ...nodeConfigOverrides }, + config: { objectName: 'crm_opportunity', filter: { id: { dialect: 'cel', source: 'recordId' } }, fields, ...nodeConfigOverrides }, }, { id: 'end', type: 'end' }, ], @@ -511,7 +511,7 @@ describe('validateReadonlyFlowWrites', () => { expect(validateReadonlyFlowWrites({ objects: [platformObject], flows: [create] })).toEqual([]); const update = { ...create, - nodes: [{ id: 'u', type: 'update_record', label: 'U', config: { objectName: platformObject.name, filter: { id: '{id}' }, fields: { verdict: 'ok' } } }], + nodes: [{ id: 'u', type: 'update_record', label: 'U', config: { objectName: platformObject.name, filter: { id: { dialect: 'cel', source: 'id' } }, fields: { verdict: 'ok' } } }], }; const control = validateReadonlyFlowWrites({ objects: [platformObject], flows: [update] }); expect(control).toHaveLength(1); diff --git a/packages/metadata-protocol/src/runtime-authoring-gate.flow-filter-operand.test.ts b/packages/metadata-protocol/src/runtime-authoring-gate.flow-filter-operand.test.ts new file mode 100644 index 00000000000..acdadbc6d9f --- /dev/null +++ b/packages/metadata-protocol/src/runtime-authoring-gate.flow-filter-operand.test.ts @@ -0,0 +1,88 @@ +// Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license. + +/** + * The SAVE door's half of the CRUD filter operands (#19939, stage S6). + * + * Each operand of a `get_record` / `update_record` / `delete_record` node's + * `filter` is a value slot: a CEL value envelope evaluated before the query, a + * query placeholder the query engine resolves, or a literal. A flow `{…}` + * token at an operand is refused — and this file asks the host's question: + * does the shipped gate a Studio / REST `/meta` / MCP author's flow publish + * passes through refuse it, with the door's own envelope (`code`, `status`, + * a located issue), and let the envelope and the placeholder through? + * + * Harness: none. {@link evaluateRuntimeAuthoringGate} is pure by construction + * (its impure reads arrive as arguments), so nothing about the door is stubbed. + */ +import { describe, expect, it } from 'vitest'; +import { VALUE_SLOT_TEMPLATE_REFUSAL } from '@objectstack/spec/automation'; + +import { evaluateRuntimeAuthoringGate } from './runtime-authoring-gate.js'; + +const CEL = (source: string) => ({ dialect: 'cel', source }); + +/** start → one CRUD node with `filter` → end, binding the variable the operands read. */ +const crudFlow = (name: string, type: string, filter: Record) => ({ + name, + label: 'Door filter probe', + type: 'autolaunched', + variables: [{ name: 'owner_id', type: 'text', isInput: true }], + nodes: [ + { id: 'start', type: 'start', label: 'Start' }, + { + id: 'op', + type, + label: 'Op', + config: { objectName: 'deal', filter, ...(type === 'update_record' ? { fields: { stage: 'closed' } } : {}) }, + }, + { id: 'end', type: 'end', label: 'End' }, + ], + edges: [ + { id: 'e1', source: 'start', target: 'op' }, + { id: 'e2', source: 'op', target: 'end' }, + ], +}); + +const save = (body: { name: string }) => + evaluateRuntimeAuthoringGate({ + type: 'flow', + name: body.name, + state: 'active', + body, + objects: [], + permissions: [], + books: [], + datasets: [], + }); + +const TYPES = ['get_record', 'update_record', 'delete_record'] as const; + +describe('the save door refuses a flow token at a CRUD filter operand (#19939)', () => { + it.each(TYPES.flatMap((type) => [ + [type, 'a field\'s value', { owner: '{owner_id}' }, 'config.filter.owner'], + [type, 'an `$in` element', { id: { $in: ['d1', '{owner_id}'] } }, 'config.filter.id.$in[1]'], + ] as const))('%s — %s: INVALID_METADATA / 422, the issue located at the operand with its envelope', (type, _what, filter, at) => { + const verdict = save(crudFlow(`door_filter_${type}`, type, filter)); + + expect(verdict.error).not.toBeNull(); + const err = verdict.error as Error & { code?: string; status?: number; issues?: Array<{ rule: string; path?: string; message: string }> }; + expect(err.code).toBe('INVALID_METADATA'); + expect(err.status).toBe(422); + const issues = (err.issues ?? []).filter((i) => i.message.startsWith(VALUE_SLOT_TEMPLATE_REFUSAL)); + expect(issues).toHaveLength(1); + expect(issues[0]!.rule).toBe('expression-invalid'); + expect(JSON.stringify(issues[0])).toContain(at); + expect(issues[0]!.message).toContain("{ dialect: 'cel', source: 'owner_id' }: an operand envelope is evaluated before the query."); + }); + + it.each(TYPES)('%s: an envelope operand, a literal and a query placeholder pass the door', (type) => { + const verdict = save(crudFlow(`door_filter_ok_${type}`, type, { + owner: CEL('owner_id'), + id: { $in: [CEL('owner_id'), 'd2'] }, + creator: '{current_user_id}', + due: { $lt: '{today}' }, + status: 'open', + })); + expect(verdict.error).toBeNull(); + }); +}); diff --git a/packages/plugins/plugin-approvals/src/status-mirror-cascade.integration.test.ts b/packages/plugins/plugin-approvals/src/status-mirror-cascade.integration.test.ts index 16811ba3b5a..70d65c16188 100644 --- a/packages/plugins/plugin-approvals/src/status-mirror-cascade.integration.test.ts +++ b/packages/plugins/plugin-approvals/src/status-mirror-cascade.integration.test.ts @@ -262,7 +262,7 @@ const onApprovedFlow = { }, { id: 'stamp', type: 'update_record', label: 'Stamp', - config: { objectName: 'opportunity', filter: { id: '{record.id}' }, fields: { cascaded: 'yes' } }, + config: { objectName: 'opportunity', filter: { id: { dialect: 'cel', source: 'record.id' } }, fields: { cascaded: 'yes' } }, }, { id: 'end', type: 'end', label: 'End' }, ], diff --git a/packages/qa/dogfood/test/declarative-endpoint-anonymous-guest.dogfood.test.ts b/packages/qa/dogfood/test/declarative-endpoint-anonymous-guest.dogfood.test.ts index 6ec9612e070..641e1dadbd4 100644 --- a/packages/qa/dogfood/test/declarative-endpoint-anonymous-guest.dogfood.test.ts +++ b/packages/qa/dogfood/test/declarative-endpoint-anonymous-guest.dogfood.test.ts @@ -67,7 +67,7 @@ const ReadFlow: Flow = { id: 'get', type: 'get_record', label: 'Get note', - config: { objectName: OBJECT, filter: { id: '{noteId}' }, outputVariable: 'found' }, + config: { objectName: OBJECT, filter: { id: { dialect: 'cel', source: 'noteId' } }, outputVariable: 'found' }, }, { id: 'end', type: 'end', label: 'End' }, ], diff --git a/packages/qa/dogfood/test/fixtures/flow-durable-suspend-fixture.ts b/packages/qa/dogfood/test/fixtures/flow-durable-suspend-fixture.ts index eba36f78c72..7079d17bf88 100644 --- a/packages/qa/dogfood/test/fixtures/flow-durable-suspend-fixture.ts +++ b/packages/qa/dogfood/test/fixtures/flow-durable-suspend-fixture.ts @@ -77,7 +77,7 @@ export const flowDurableSuspend: Flow = { label: 'Apply resolution', config: { objectName: 'suspend_note', - filter: { id: '{noteId}' }, + filter: { id: { dialect: 'cel', source: 'noteId' } }, // A CEL value envelope — the `{…}` template dialect is retired from // value slots; `resolution` is the screen's required field. fields: { status: 'resolved', resolution: { dialect: 'cel', source: 'resolution' } }, diff --git a/packages/qa/dogfood/test/fixtures/flow-runas-fixture.ts b/packages/qa/dogfood/test/fixtures/flow-runas-fixture.ts index 45649a62cea..d27509b2207 100644 --- a/packages/qa/dogfood/test/fixtures/flow-runas-fixture.ts +++ b/packages/qa/dogfood/test/fixtures/flow-runas-fixture.ts @@ -73,7 +73,7 @@ function touchFlow(name: string, runAs: 'system' | 'user', stamp: string): Flow id: 'touch', type: 'update_record', label: 'Touch note', - config: { objectName: 'runas_note', filter: { id: '{noteId}' }, fields: { status: stamp } }, + config: { objectName: 'runas_note', filter: { id: { dialect: 'cel', source: 'noteId' } }, fields: { status: stamp } }, }, { id: 'end', type: 'end', label: 'End' }, ], @@ -106,7 +106,7 @@ function readFlow(name: string, runAs: 'system' | 'user'): Flow { id: 'get', type: 'get_record', label: 'Get note', - config: { objectName: 'runas_note', filter: { id: '{noteId}' }, outputVariable: 'found' }, + config: { objectName: 'runas_note', filter: { id: { dialect: 'cel', source: 'noteId' } }, outputVariable: 'found' }, }, { id: 'end', type: 'end', label: 'End' }, ], diff --git a/packages/qa/dogfood/test/fixtures/flow-touch-fixture.ts b/packages/qa/dogfood/test/fixtures/flow-touch-fixture.ts index 0c15823c285..ee3c0981f1d 100644 --- a/packages/qa/dogfood/test/fixtures/flow-touch-fixture.ts +++ b/packages/qa/dogfood/test/fixtures/flow-touch-fixture.ts @@ -56,7 +56,7 @@ export const flowTouch: Flow = { label: 'Mark processed', config: { objectName: 'flow_note', - filter: { id: '{noteId}' }, + filter: { id: { dialect: 'cel', source: 'noteId' } }, fields: { status: 'processed' }, }, }, diff --git a/packages/qa/dogfood/test/fixtures/schedule-organization-fixture.ts b/packages/qa/dogfood/test/fixtures/schedule-organization-fixture.ts index 27a99f072e3..8e50ee89ad5 100644 --- a/packages/qa/dogfood/test/fixtures/schedule-organization-fixture.ts +++ b/packages/qa/dogfood/test/fixtures/schedule-organization-fixture.ts @@ -172,7 +172,7 @@ export function declaringTimeRelativeFlow(organizationId: string, recipientId: s label: 'Mark touched', config: { objectName: 'sched_org_target', - filter: { id: '{record.id}' }, + filter: { id: { dialect: 'cel', source: 'record.id' } }, fields: { touched: true }, }, }, diff --git a/packages/services/service-automation/README.md b/packages/services/service-automation/README.md index c62c1b3867d..1a1cbd68396 100644 --- a/packages/services/service-automation/README.md +++ b/packages/services/service-automation/README.md @@ -110,7 +110,7 @@ const escalateCase = { label: 'Load Owner', config: { objectName: 'sys_user', - filter: { id: '{record.owner_id}' }, + filter: { id: { dialect: 'cel', source: 'record.owner_id' } }, fields: ['id', 'name', 'email'], outputVariable: 'owner', }, @@ -121,7 +121,7 @@ const escalateCase = { label: 'Flag Case', config: { objectName: 'crm_case', - filter: { id: '{record.id}' }, + filter: { id: { dialect: 'cel', source: 'record.id' } }, // A computed field value is a CEL value envelope; a literal is written as it is. fields: { escalated: true, escalation_note: { dialect: 'cel', source: "'Escalated to ' + owner.name" } }, }, @@ -169,10 +169,14 @@ one this README used to get wrong: | `update_record` | `objectName`, `filter`, `fields` — **no** `outputVariable`; the executor does not read one | | `delete_record` | `objectName`, `filter` | -`filter` is an **object** of field/value pairs (`{ id: '{record.id}' }`), not an -array of `{ field, operator, value }` triples; operator objects such as -`{ "$ne": null }` are legal values. There is no `recordId` key — select by id -through `filter`. Unknown keys are rejected at `registerFlow()`. +`filter` is an **object** of field/value pairs +(`{ id: { dialect: 'cel', source: 'record.id' } }`), not an array of +`{ field, operator, value }` triples; operator objects such as +`{ "$ne": null }` are legal values. Each operand is a CEL value envelope, +evaluated before the query (one that evaluates to `null` refuses the node), a +query placeholder the query engine resolves (`{current_user_id}`, `{today}`), +or a literal. There is no `recordId` key — select by id through `filter`. +Unknown keys are rejected at `registerFlow()`. For every other node's `config`, and for loops, parallel blocks, subflows, waits and error handling, see the maintained reference — **[Flows](https://objectstack.ai/docs/automation/flows)**. @@ -189,6 +193,7 @@ a CEL value envelope, and text is a `{{ }}` template.** | Edge `condition` | CEL — bare, no braces | `record.status == 'open'` | | Decision `conditions[].expression` | CEL — bare, no braces | `order_amount > 10000` | | Value slots — `create_record` / `update_record` `fields`, `assignment` values, the maps handed to a callee: `subflow` / `map` `input`, `script` `inputs`, a `screen`'s `defaults`, field `defaultValue` and `recordId`, each evaluated before the screen is served, a `loop`'s (with a `body`) or `map`'s `collection`, a `notify` node's `recipients` (each entry of a list), `templateData` values and `sourceId` (its `sourceObject`, `actorId` and `payload` are literals, read as written; a `{…}` token there is refused), and an `http` node's `body`, evaluated once before the call (its `method`, `durable`, `timeoutMs` and `signingSecret` are literals, read as written; a `{…}` token there is refused) | CEL value envelope; a plain string is the literal text it spells (`recordId`'s literal is the id, a string; a `collection`'s literal is an inline array, and any string there is refused; a recipient entry that evaluates to a list adds each of its recipients) | `{ dialect: 'cel', source: "'Follow up on ' + record.name" }`, `'open'`, `collection: { dialect: 'cel', source: 'tasks' }` | +| A `get_record` / `update_record` / `delete_record` `filter` operand — a field's value, an operator's comparand, a list comparand's element | CEL value envelope, evaluated before the query (`null` refuses the node); or a literal — a query placeholder (`{current_user_id}`, `{today}`) among them, handed to the query engine untouched; any other `{…}` token is refused | `{ id: { dialect: 'cel', source: 'record.id' }, due_date: { $lt: '{today}' } }` | | Text slots — `notify` `title` / `message` / `actionUrl`, `http` `url` and each `headers` value, `screen` `title` / `description`, `end` `message` | Template — `{{ }}` holes (ADR-0032 §3; a single-brace token is refused) | `'Deal won: {{ record.name }}'`, `'{{ record.amount \| currency }}'` | A value slot refuses every `{…}` template token — `registerFlow()` and @@ -208,8 +213,7 @@ Inside an object or list value, and in the legacy `assignment` shapes, an envelope is data: build the whole value as one envelope (a CEL map or list literal, its mixed values wrapped in `dyn(…)`), or move the assignment into the `assignments` map — an `http` `body` included: `{ dialect: 'cel', source: -"{'id': record.id, 'text': dyn('Done: ' + record.title)}" }`. A `filter` still reads -the single-brace dialect (`{record.id}`). +"{'id': record.id, 'text': dyn('Done: ' + record.title)}" }`. A `loop`'s or a `map`'s `collection` must evaluate to a list: anything else, `null` included, fails the node, as a token that resolved to nothing did, so a diff --git a/packages/services/service-automation/src/builtin/crud-bulk-intent.test.ts b/packages/services/service-automation/src/builtin/crud-bulk-intent.test.ts index 10c26eacfc3..5a8ceb524f5 100644 --- a/packages/services/service-automation/src/builtin/crud-bulk-intent.test.ts +++ b/packages/services/service-automation/src/builtin/crud-bulk-intent.test.ts @@ -186,7 +186,7 @@ describe('delete_record — bulk intent reaches deleteMany, and only when declar it('a scalar-id delete still needs no declaration — the by-id route is untouched', async () => { engine.registerFlow('bulk_flow', crudFlow('delete_record', { objectName: 'deal', - filter: { id: '{record.id}' }, + filter: { id: { dialect: 'cel', source: 'record.id' } }, }) as never); const result = await engine.execute('bulk_flow', { record: { id: 'd2' } } as never); @@ -242,16 +242,19 @@ describe('delete_record — bulk intent reaches deleteMany, and only when declar it('declaring `multi` does NOT disarm the #3810 erased-condition guard', async () => { // The two guards are independent: bulk intent says "many rows are // fine", it never says "a condition the author wrote may vanish". + // [#19939, stage S6] The vanishing condition is an operand envelope + // that evaluates to nothing (the record has no owner); it was a + // `{record.ownr}` token, which the contract parse refuses since S6. engine.registerFlow('bulk_flow', crudFlow('delete_record', { objectName: 'deal', - filter: { owner: '{record.ownr}' }, + filter: { owner: { dialect: 'cel', source: 'record.owner' } }, multi: true, }) as never); - const result = await engine.execute('bulk_flow', { record: { id: 'd1', owner: 'usr_7' } } as never); + const result = await engine.execute('bulk_flow', { record: { id: 'd1', owner: null } } as never); expect(result.success).toBe(false); - expect(result.error).toContain('{record.ownr}'); + expect(result.error).toContain('`record.owner` (at filter.owner)'); expect(data.calls.delete).toBeUndefined(); expect(data.remaining()).toHaveLength(3); }); @@ -302,7 +305,7 @@ describe('update_record — the executor forwards the declared bulk intent (#539 it('a by-id update is unchanged — `multi: false`, one row, `acted: 1`', async () => { engine.registerFlow('bulk_flow', crudFlow('update_record', { objectName: 'deal', - filter: { id: '{record.id}' }, + filter: { id: { dialect: 'cel', source: 'record.id' } }, fields: { stage: 'won' }, }) as never); diff --git a/packages/services/service-automation/src/builtin/crud-filter-guard.test.ts b/packages/services/service-automation/src/builtin/crud-filter-guard.test.ts index 509031c8c81..adaeb8d4f5d 100644 --- a/packages/services/service-automation/src/builtin/crud-filter-guard.test.ts +++ b/packages/services/service-automation/src/builtin/crud-filter-guard.test.ts @@ -1,31 +1,41 @@ // Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license. import { describe, it, expect, beforeEach } from 'vitest'; +import { VALUE_SLOT_TEMPLATE_REFUSAL } from '@objectstack/spec/automation'; import { AutomationEngine } from '../engine.js'; import { registerCrudNodes } from './crud-nodes.js'; -import { interpolateFilter } from './template.js'; import { assertEngineFindOnePredicate } from '@objectstack/metadata-core'; /** - * framework#3810 — a flow filter must never silently widen. + * A CRUD node's `filter` — its operands are value slots, and it never widens + * silently (#19939 stage S6, Fork R17 → A; framework#3810). * - * The flow interpolator expresses "this token did not resolve" as `undefined`. - * Everywhere else that is harmless; in a FILTER it removes a condition, and a - * removed condition matches MORE rows. When it was the only condition, - * `{ owner: '{record.ownr}' }` became `{}` — and `{}` handed to `deleteMany` is - * every row in the table. One mistyped field name emptied the object, silently. + * Each OPERAND of a `get_record` / `update_record` / `delete_record` filter — + * a field's value, an operator's comparand, each element of a list comparand — + * is a CEL value envelope the executor evaluates in the run's scope BEFORE the + * query, or a literal handed to the query as written. A query placeholder + * (`{current_user_id}`, `{current_year_start}`) is a literal of the query + * engine's own vocabulary, handed on untouched for it to resolve. A flow + * `{…}` token is refused at every door. * - * Two independent fixes are covered here: - * 1. Filter placeholders (`{current_user_id}`, `{current_year_start}`) are - * passed through to the query engine, which owns that dialect. - * 2. Anything else that fails to resolve — a typo, a missing input, a lookup - * hop — refuses the node instead of executing a widened query. + * The #3810 guard holds for an envelope that evaluates to nothing: a condition + * whose comparand is `null` or nothing is not a narrower query, it is a wider + * one — `{ owner: }` handed to `deleteMany` was every row in the + * table. So an operand envelope that evaluates to `null` REFUSES the node, as a + * guard (a `fault` edge may not swallow it), before any query. + * + * Before S6, the executor interpolated the filter with the single-brace + * dialect (`interpolateFilter`): a flow token resolved against the run's + * variables, an unresolved one was erased and refused here, and a placeholder + * no flow variable shadowed was handed on. */ function createTestLogger(): any { return { info: () => {}, warn: () => {}, error: () => {}, debug: () => {}, child: () => createTestLogger() }; } +const CEL = (source: string) => ({ dialect: 'cel', source }); + /** A data engine stub that records the `where` each verb actually received. */ function fakeData() { const seen: { find?: any; update?: any; delete?: any } = {}; @@ -56,8 +66,8 @@ function crudFlow(nodeType: string, config: Record) { label: 'CRUD Flow', type: 'autolaunched' as const, // Explicit so the ADR-0049 runAs gate (#3760) does not reject the data - // op first — these tests are about the filter guard, and a refusal for - // the wrong reason would make the negative cases pass vacuously. + // op first — these tests are about the filter, and a refusal for the + // wrong reason would make the negative cases pass vacuously. runAs: 'system' as const, nodes: [ { id: 'start', type: 'start' as const, label: 'Start' }, @@ -71,44 +81,108 @@ function crudFlow(nodeType: string, config: Record) { }; } -describe('interpolateFilter — filter placeholders are handed to the engine (#3810)', () => { - const vars = new Map([['record', { id: 'r1', owner: 'usr_7' }]]); - const ctx = { userId: 'usr_1' } as any; +/** Each node type, with the verb its data call is recorded under and the `fields` an update needs. */ +const NODES = [ + ['get_record', 'find', {}], + ['update_record', 'update', { fields: { title: 'x' } }], + ['delete_record', 'delete', {}], +] as const; + +describe('a filter operand envelope is evaluated before the query; a query placeholder is handed on (#19939)', () => { + let engine: AutomationEngine; + let data: ReturnType; - it('passes a date macro through verbatim', () => { - expect(interpolateFilter({ close_date: { $gte: '{current_year_start}' } }, vars, ctx)) - .toEqual({ close_date: { $gte: '{current_year_start}' } }); + beforeEach(() => { + data = fakeData(); + engine = new AutomationEngine(createTestLogger()); + registerCrudNodes(engine, ctxWith(data.service)); }); - it('passes a parameterised macro and a context token through verbatim', () => { - expect(interpolateFilter({ a: '{30_days_ago}', b: '{current_user_id}' }, vars, ctx)) - .toEqual({ a: '{30_days_ago}', b: '{current_user_id}' }); + it.each(NODES)('%s: every operand position evaluates — a field\'s value, a comparand, a list element, a combinator member, a nested relation', async (nodeType, verb, extra) => { + engine.registerFlow('crud_flow', crudFlow(nodeType, { + objectName: 'deal', + filter: { + owner: CEL('record.owner'), + amount: { $gt: CEL('record.amount * 2'), $lte: 1000 }, + id: { $in: ['d0', CEL('record.id')] }, + $or: [{ stage: CEL("record.stage + '_review'") }, { stage: 'won' }], + account: { region: CEL('record.region') }, + }, + ...extra, + })); + const record = { id: 'd1', owner: 'usr_7', amount: 50, stage: 'legal', region: 'emea' }; + const result = await engine.execute('crud_flow', { record } as any); + + expect(result.success).toBe(true); + expect(data.seen[verb]).toEqual({ + owner: 'usr_7', + amount: { $gt: 100, $lte: 1000 }, + id: { $in: ['d0', 'd1'] }, + $or: [{ stage: 'legal_review' }, { stage: 'won' }], + account: { region: 'emea' }, + }); }); - it('still resolves flow variables', () => { - expect(interpolateFilter({ owner: '{record.owner}' }, vars, ctx)).toEqual({ owner: 'usr_7' }); + it.each(NODES)('%s: a query placeholder reaches the engine untouched, beside a literal', async (nodeType, verb, extra) => { + engine.registerFlow('crud_flow', crudFlow(nodeType, { + objectName: 'deal', + filter: { close_date: { $lt: '{current_year_start}' }, a: '{30_days_ago}', b: '{current_user_id}', status: 'open' }, + ...extra, + })); + const result = await engine.execute('crud_flow', { record: { id: 'r1' } } as any); + + expect(result.success).toBe(true); + expect(data.seen[verb]).toEqual({ + close_date: { $lt: '{current_year_start}' }, a: '{30_days_ago}', b: '{current_user_id}', status: 'open', + }); }); - it('mixes both dialects in one filter', () => { - expect( - interpolateFilter({ owner: '{record.owner}', close_date: { $gte: '{current_year_start}' } }, vars, ctx), - ).toEqual({ owner: 'usr_7', close_date: { $gte: '{current_year_start}' } }); + it('a flow variable named like a placeholder no longer shadows it — the variable is read through an envelope', async () => { + // Before S6 a flow variable named `current_user_id` won over the + // placeholder at `{current_user_id}`. No flow token is read at a filter + // operand now: the placeholder is the query engine's, and the variable + // is `{ dialect: 'cel', source: 'current_user_id' }`. + engine.registerFlow('crud_flow', { + ...crudFlow('delete_record', { objectName: 'deal', filter: { a: '{current_user_id}', b: CEL('current_user_id') } }), + variables: [{ name: 'current_user_id', type: 'text', isInput: true }], + }); + const result = await engine.execute('crud_flow', { params: { current_user_id: 'from_flow_var' } } as any); + + expect(result.success).toBe(true); + expect(data.seen.delete).toEqual({ a: '{current_user_id}', b: 'from_flow_var' }); }); - it('a flow variable still shadows a same-named placeholder', () => { - // Precedence guard: a template that resolves today must not start - // meaning something else. - const shadowed = new Map([['current_user_id', 'from_flow_var']]); - expect(interpolateFilter({ owner: '{current_user_id}' }, shadowed, ctx)) - .toEqual({ owner: 'from_flow_var' }); + it('the node\'s config is never written into — the resolved filter is a copy', async () => { + const filter = { owner: CEL('record.owner') }; + engine.registerFlow('crud_flow', crudFlow('delete_record', { objectName: 'deal', filter })); + await engine.execute('crud_flow', { record: { owner: 'usr_7' } } as any); + + expect(data.seen.delete).toEqual({ owner: 'usr_7' }); + expect(filter).toEqual({ owner: CEL('record.owner') }); }); - it('leaves a non-placeholder unresolved token as undefined for the guard to catch', () => { - expect(interpolateFilter({ owner: '{record.ownr}' }, vars, ctx)).toEqual({ owner: undefined }); + it('an intentionally empty filter is still allowed (nothing was erased)', async () => { + // "Delete everything" must stay expressible — the guard fires on an + // operand that is nothing, not on emptiness, so an author who wrote no + // filter is unaffected. + engine.registerFlow('crud_flow', crudFlow('delete_record', { objectName: 'deal', filter: {} })); + + const result = await engine.execute('crud_flow'); + + expect(result.success).toBe(true); + expect(data.seen.delete).toEqual({}); + }); + + it('a literal `null` operand is an authored IS NULL condition, handed on — only a COMPUTED nothing is refused', async () => { + engine.registerFlow('crud_flow', crudFlow('delete_record', { objectName: 'deal', filter: { owner: null } })); + const result = await engine.execute('crud_flow'); + + expect(result.success).toBe(true); + expect(data.seen.delete).toEqual({ owner: null }); }); }); -describe('crud nodes — refuse a filter that lost a condition (#3810)', () => { +describe('crud nodes — refuse a filter whose operand evaluated to nothing (#3810)', () => { let engine: AutomationEngine; let data: ReturnType; @@ -118,91 +192,137 @@ describe('crud nodes — refuse a filter that lost a condition (#3810)', () => { registerCrudNodes(engine, ctxWith(data.service)); }); - it('delete_record: a typo\'d field name refuses instead of emptying the object', async () => { - engine.registerFlow('crud_flow', crudFlow('delete_record', { - objectName: 'deal', - filter: { owner: '{record.ownr}' }, - })); + async function runRefused(nodeType: string, config: Record, record: Record) { + engine.registerFlow('crud_flow', crudFlow(nodeType, config)); + const result = await engine.execute('crud_flow', { record } as any); + const runs = await engine.listRuns('crud_flow'); + const step = runs[0]?.steps.find((s) => s.nodeId === 'op'); + return { result, step }; + } - const result = await engine.execute('crud_flow', { record: { id: 'r1', owner: 'usr_7' } } as any); + it('delete_record: an envelope that evaluates to null refuses the node instead of emptying the object', async () => { + const { result, step } = await runRefused('delete_record', { + objectName: 'deal', + filter: { owner: CEL('record.owner') }, + }, { id: 'r1', owner: null }); expect(result.success).toBe(false); - expect(result.error).toContain('{record.ownr}'); - expect(result.error).toMatch(/deleted it|every remaining row/); + expect(step?.error?.code).toBe('NODE_FAILURE'); + expect(result.error).toContain('delete_record: refusing to run — 1 filter operand(s) evaluated to nothing'); + expect(result.error).toContain('`record.owner` (at filter.owner)'); + expect(result.error).toContain('would have matched every remaining row and deleted it'); expect(data.seen.delete).toBeUndefined(); }); - it('update_record: a missing input variable refuses instead of overwriting everything', async () => { - engine.registerFlow('crud_flow', crudFlow('update_record', { + it('update_record: an envelope that evaluates to null refuses the node instead of overwriting the rows', async () => { + const { result, step } = await runRefused('update_record', { objectName: 'deal', - filter: { owner: '{someInputNeverSet}' }, + filter: { owner: CEL('record.owner') }, fields: { title: 'x' }, - })); - - const result = await engine.execute('crud_flow'); + }, { id: 'r1', owner: null }); expect(result.success).toBe(false); - expect(result.error).toContain('{someInputNeverSet}'); + expect(step?.error?.code).toBe('NODE_FAILURE'); + expect(result.error).toContain('update_record: refusing to run'); + expect(result.error).toContain('`record.owner` (at filter.owner)'); expect(data.seen.update).toBeUndefined(); }); - it('refuses when only SOME conditions are lost — a partial filter still widens', async () => { - engine.registerFlow('crud_flow', crudFlow('delete_record', { + it('get_record: an envelope that evaluates to null refuses the read', async () => { + const { result, step } = await runRefused('get_record', { objectName: 'deal', - filter: { status: 'open', owner: '{record.ownr}' }, - })); - - const result = await engine.execute('crud_flow', { record: { id: 'r1', owner: 'usr_7' } } as any); + filter: { owner: CEL('record.owner') }, + }, { owner: null }); expect(result.success).toBe(false); - expect(data.seen.delete).toBeUndefined(); + expect(step?.error?.code).toBe('NODE_FAILURE'); + expect(result.error).toContain('get_record: refusing to run'); + expect(data.seen.find).toBeUndefined(); }); - it('names the lookup hop and points at config.expand', async () => { - engine.registerFlow('crud_flow', crudFlow('delete_record', { - objectName: 'deal', - filter: { owner: '{record.account.name}' }, - })); - - const result = await engine.execute('crud_flow', { record: { id: 'r1', account: 'acc_1' } } as any); + it.each([ + ['a guarded form answering null', { owner: CEL('has(record.owner) ? record.owner : null') }, 'filter.owner'], + ['an operator\'s comparand', { amount: { $gt: CEL('record.cap') } }, 'filter.amount.$gt'], + ['an `$in` element', { id: { $in: ['r1', CEL('record.cap')] } }, 'filter.id.$in[1]'], + ['a `$or` member\'s value', { $or: [{ status: 'open' }, { owner: CEL('record.cap') }] }, 'filter.$or[1].owner'], + ])('delete_record: %s that evaluates to nothing refuses the node — located at the operand', async (_label, filter, at) => { + const { result } = await runRefused('delete_record', { objectName: 'deal', filter }, { id: 'r1', cap: null }); expect(result.success).toBe(false); - expect(result.error).toContain('expand'); + expect(result.error).toContain(`(at ${at})`); + expect(data.seen.delete).toBeUndefined(); }); - it('a filter placeholder is NOT treated as erased — it reaches the engine', async () => { - engine.registerFlow('crud_flow', crudFlow('delete_record', { + it('refuses when only SOME operands are nothing — a partial filter still widens', async () => { + const { result } = await runRefused('delete_record', { objectName: 'deal', - filter: { close_date: { $lt: '{current_year_start}' } }, - })); + filter: { status: 'open', owner: CEL('record.owner') }, + }, { id: 'r1', owner: null }); - const result = await engine.execute('crud_flow', { record: { id: 'r1' } } as any); - - expect(result.success).toBe(true); - // Handed on verbatim; the query engine owns the expansion. - expect(data.seen.delete).toEqual({ close_date: { $lt: '{current_year_start}' } }); + expect(result.success).toBe(false); + expect(data.seen.delete).toBeUndefined(); }); - it('a fully-resolvable filter still runs untouched', async () => { - engine.registerFlow('crud_flow', crudFlow('delete_record', { + it('an absent variable or key fails the run with an evaluation fault naming it — no query runs', async () => { + const { result, step } = await runRefused('delete_record', { objectName: 'deal', - filter: { owner: '{record.owner}' }, - })); - - const result = await engine.execute('crud_flow', { record: { id: 'r1', owner: 'usr_7' } } as any); + filter: { owner: CEL('record.ownr') }, + }, { id: 'r1', owner: 'usr_7' }); - expect(result.success).toBe(true); - expect(data.seen.delete).toEqual({ owner: 'usr_7' }); + expect(result.success).toBe(false); + expect(step?.error?.code).toBe('EXECUTION_ERROR'); + expect(step?.error?.message).toContain('filter.owner: value expression failed to evaluate as CEL'); + expect(data.seen.delete).toBeUndefined(); }); +}); - it('an intentionally empty filter is still allowed (nothing was erased)', async () => { - // "Delete everything" must stay expressible — the guard fires on LOSS, - // not on emptiness, so an author who wrote no filter is unaffected. - engine.registerFlow('crud_flow', crudFlow('delete_record', { objectName: 'deal', filter: {} })); +describe('a flow `{…}` token at a filter operand is refused at every door (#19939)', () => { + /** The builtin CRUD executor itself — the run-time door, reached without `registerFlow`. */ + function executorOf(nodeType: string, data: ReturnType) { + const executors = new Map Promise }>(); + const engine = new AutomationEngine(createTestLogger()); + const recorder = new Proxy(engine, { + get(target, key, receiver) { + if (key === 'registerNodeExecutor') return (e: any) => { executors.set(e.type, e); }; + return Reflect.get(target, key, receiver); + }, + }); + registerCrudNodes(recorder as AutomationEngine, ctxWith(data.service)); + return executors.get(nodeType)!; + } + + it.each(NODES)('%s: `registerFlow` refuses a token, located at the operand, naming its envelope', (nodeType, _verb, extra) => { + const engine = new AutomationEngine(createTestLogger()); + registerCrudNodes(engine, ctxWith(fakeData().service)); + expect(() => engine.registerFlow('crud_flow', crudFlow(nodeType, { + objectName: 'deal', filter: { owner: '{record.owner}', due: { $lt: '{today}' } }, ...extra, + }))).toThrow(`${nodeType} filter operand at config.filter.owner: ${VALUE_SLOT_TEMPLATE_REFUSAL}`); + }); - const result = await engine.execute('crud_flow'); + it.each(NODES)('%s: the executor\'s contract parse refuses it as a guard, and nothing is queried', async (nodeType, verb, extra) => { + const data = fakeData(); + const result = await executorOf(nodeType, data).execute( + { id: 'op', type: nodeType, label: 'Op', config: { objectName: 'deal', filter: { owner: '{record.owner}' }, ...extra } }, + new Map([['record', { owner: 'usr_7' }]]), + {}, + ); + expect(result.success).toBe(false); + expect(result.errorClass).toBe('guard'); + expect(result.error).toContain(VALUE_SLOT_TEMPLATE_REFUSAL); + expect(result.error).toContain("{ dialect: 'cel', source: 'record.owner' }"); + expect(data.seen[verb]).toBeUndefined(); + }); - expect(result.success).toBe(true); - expect(data.seen.delete).toEqual({}); + it.each(NODES)('%s: the executor\'s guard refusal for an operand that evaluated to nothing is a guard too', async (nodeType, verb, extra) => { + const data = fakeData(); + const result = await executorOf(nodeType, data).execute( + { id: 'op', type: nodeType, label: 'Op', config: { objectName: 'deal', filter: { owner: CEL('record.owner') }, ...extra } }, + new Map([['record', { owner: null }]]), + {}, + ); + expect(result.success).toBe(false); + expect(result.errorClass).toBe('guard'); + expect(result.error).toContain('filter operand(s) evaluated to nothing'); + expect(data.seen[verb]).toBeUndefined(); }); }); diff --git a/packages/services/service-automation/src/builtin/crud-nodes.ts b/packages/services/service-automation/src/builtin/crud-nodes.ts index 7dd7d6e4b75..01e16fa3c44 100644 --- a/packages/services/service-automation/src/builtin/crud-nodes.ts +++ b/packages/services/service-automation/src/builtin/crud-nodes.ts @@ -3,6 +3,8 @@ import type { PluginContext } from '@objectstack/core'; import { defineActionDescriptor, + flowFilterOperands, + isExpressionEnvelopeShaped, GetRecordConfigSchema, CreateRecordConfigSchema, UpdateRecordConfigSchema, @@ -29,94 +31,110 @@ import { type StoredHashDigest, } from '@objectstack/metadata-protocol'; import type { AutomationEngine } from '../engine.js'; -import { interpolateFilter, type VariableMap } from './template.js'; +import type { VariableMap } from './template.js'; import { resolveValueSlotMap } from './value-slot-map.js'; import { refuseNode } from '../guard-refusal.js'; import { parseNodeConfig } from './parse-config.js'; import { resolveRunDataContext, stampSystemInsertOwner } from '../runtime-identity.js'; /** - * A filter condition that an author WROTE but that interpolation erased - * (framework#3810). + * Resolve a node's `filter` to the one the query runs (#19939, Fork R17 → A; + * ADR-0032 Decision 2: a computed filter value is a CEL envelope). * - * The flow interpolator expresses "this token did not resolve" as `undefined`. - * In every other config block that is harmless — an unresolved `{x}` in a - * message renders as empty text. In a FILTER it is the opposite of harmless: - * a condition whose value is `undefined` is not a narrower query, it is an - * ABSENT one, and an absent condition matches MORE rows. When the erased - * condition was the only one, `{ owner: '{record.ownr}' }` becomes `{}` — and - * `{}` handed to `deleteMany` is every row in the table. + * Each OPERAND of the filter — a field's value, an operator's comparand, each + * element of a list comparand — is a value slot, located by the spec's + * `flowFilterOperands`, the walk every door reads, so "which positions the + * doors judge" and "which positions the executor evaluates" cannot drift + * apart. An operand that is a CEL value envelope is EVALUATED by + * `AutomationEngine.evaluateValueEnvelope` in the run's scope and context — + * the call every value slot makes; a malformed one, or one that faults on the + * live variables (an absent variable or key), throws with its source, before + * any query. Every other operand is handed to the query as written: a literal, + * or a query placeholder (`{current_user_id}`, `{today}`) the query engine + * resolves itself. A flow `{…}` token never reaches this point: the + * executor's `parseNodeConfig` refused it through the node contract (the + * judge `registerFlow` and `objectstack validate` call). The filter is copied, + * so the node's config is never written into. * - * So a single mistyped field name in a `delete_record` node silently emptied - * the object. Not a hypothetical: `{record.ownr}` (typo), `{someInput}` (an - * input the run did not receive) and `{record.account.name}` (a lookup hop — - * the trigger record carries a scalar id) all reach this state, and none of - * them produced a diagnostic anywhere. + * **The #3810 guard, kept.** An operand envelope that evaluates to `null` (or + * to nothing) REFUSES the node. A condition whose comparand is nothing is not + * a narrower query, it is a different one — dropped, it matches MORE rows; as + * `null`, it matches every row whose field is empty. When the erased condition + * was the only one, `{}` handed to `deleteMany` is every row in the table. The + * 17.x template reached this state silently through a mistyped field name, an + * input the run did not receive or a lookup hop, until this guard refused it; + * an operand that evaluates to `null` is the same state, and it is refused + * whatever the operator. An author who means "the field is empty" writes the + * literal `null` (or `{ $null: true }`), never a computed one. The guard is + * keyed on "an operand the author wrote is nothing", not on "the filter is + * empty": losing ONE of two conditions still widens the blast radius from "my + * open records" to "all open records". * - * The guard below refuses to execute such a node. It is deliberately keyed on - * "a condition the author wrote is gone", not on "the filter is empty": losing - * ONE of two conditions still silently widens the blast radius from "my open - * records" to "all open records". - */ -function erasedFilterConditions( - before: unknown, - after: unknown, - path: string[] = [], -): Array<{ path: string; template: string }> { - const out: Array<{ path: string; template: string }> = []; - const at = (p: string[]) => p.join('.') || '(root)'; - - if (typeof before === 'string') { - if (after === undefined && before.includes('{')) { - out.push({ path: at(path), template: before }); - } - return out; - } - if (Array.isArray(before)) { - before.forEach((v, i) => - out.push(...erasedFilterConditions(v, (after as unknown[] | undefined)?.[i], [...path, String(i)])), - ); - return out; - } - if (before && typeof before === 'object') { - const afterRec = (after ?? {}) as Record; - for (const [k, v] of Object.entries(before as Record)) { - out.push(...erasedFilterConditions(v, afterRec[k], [...path, k])); - } - } - return out; -} - -/** - * Interpolate a node's filter and refuse the node if any authored condition was - * erased. Returns either the usable filter or a ready-to-return failure. - * - * `verb` names the operation in the error so the message says what WOULD have - * happened ("would have matched every row and deleted it"). + * `consequence` names the operation in the refusal, so the message says what + * WOULD have happened ("would have matched every remaining row and deleted it"). */ function resolveNodeFilter( + engine: AutomationEngine, rawFilter: unknown, variables: VariableMap, - context: Parameters[2], + context: AutomationContext, nodeType: string, consequence: string, ): { filter: Record } | { error: string } { - const filter = interpolateFilter(rawFilter ?? {}, variables, context) as Record; - const erased = erasedFilterConditions(rawFilter ?? {}, filter); - if (erased.length > 0) { - const detail = erased.map((e) => `\`${e.template}\` (at ${e.path})`).join(', '); + const authored = rawFilter ?? {}; + const filter = copyFilter(authored) as Record; + const nothing: Array<{ path: string; source: string }> = []; + for (const operand of flowFilterOperands(authored)) { + if (!isExpressionEnvelopeShaped(operand.value)) continue; + const at = filterOperandPath(operand.path); + const value = engine.evaluateValueEnvelope(operand.value, variables, at, context); + if (value === null || value === undefined) { + const source = (operand.value as { source?: unknown }).source; + nothing.push({ path: at, source: typeof source === 'string' ? source : '' }); + continue; + } + setFilterOperand(filter, operand.path, value); + } + if (nothing.length > 0) { + const detail = nothing.map((e) => `\`${e.source}\` (at ${e.path})`).join(', '); return { error: - `${nodeType}: refusing to run — ${erased.length} filter condition(s) resolved to nothing ` + - `and were dropped from the query: ${detail}. An absent condition does not narrow a query, ` + - `it widens it, so this ${consequence}. Check the field name, confirm the flow variable is ` + - `set on this run, and note that a relation field holds a scalar id — a \`{record..}\` ` + - `hop needs the relation in the start node's \`config.expand\`.`, + `${nodeType}: refusing to run — ${nothing.length} filter operand(s) evaluated to nothing: ${detail}. ` + + `An absent condition does not narrow a query, it widens it, so this ${consequence}. A filter ` + + `operand must evaluate to a value on every run that reaches this node: check the expression and ` + + `confirm the variable is set on this run, and where the value may be absent, route around this node ` + + `with a \`decision\`. To match an empty field, write the literal \`null\` as the operand.`, }; } return { filter }; } +/** A filter operand's path, in the ledger's spelling (`filter.id.$in[0]`). */ +function filterOperandPath(path: readonly (string | number)[]): string { + let out = 'filter'; + for (const segment of path) out += typeof segment === 'number' ? `[${segment}]` : `.${segment}`; + return out; +} + +/** A copy of a filter's plain objects and lists; every other value is shared, as the query only reads it. */ +function copyFilter(value: unknown): unknown { + if (Array.isArray(value)) return value.map(copyFilter); + const proto = value !== null && typeof value === 'object' ? Object.getPrototypeOf(value) : undefined; + if (proto === Object.prototype || proto === null) { + const out: Record = {}; + for (const [k, v] of Object.entries(value as Record)) out[k] = copyFilter(v); + return out; + } + return value; +} + +/** Write an evaluated operand into the copied filter, at the path `flowFilterOperands` located it. */ +function setFilterOperand(filter: Record, path: readonly (string | number)[], value: unknown): void { + let at: Record = filter; + for (const segment of path.slice(0, -1)) at = at[segment] as Record; + at[path[path.length - 1]!] = value; +} + /** * #3407 — render a data-layer strip event as a step warning. The write itself * SUCCEEDED; the warning tells the flow author which requested fields never @@ -310,10 +328,10 @@ function projectionNamingId(fields: string[] | undefined): string[] | undefined * then the content-hash refusal (`storedMetadataHashEvaluateRefusal`). * * `query` is the option bag the node hands the engine, so the collector reads - * exactly the filter the engine would run: the INTERPOLATED one, since a - * `{token}` can resolve to a whole condition (a `$and` list, a comparand) whose - * columns the authored template does not show. The node configs declare no - * sort and no grouping, so the refusals are fed filter fields only. + * exactly the filter the engine would run: the RESOLVED one, since an operand + * envelope can evaluate to a comparand (a `{ $field }` reference among them) + * whose columns the authored envelope does not show. The node configs declare + * no sort and no grouping, so the refusals are fed filter fields only. * * The answer is a guard refusal ({@link refuseNode}: the metadata is wrong, * and re-running it unchanged never succeeds) carrying the door's own error @@ -397,15 +415,17 @@ function storedMetadataWriteRefusal( * seeds them directly (ADR-0018) rather than shipping a separate plugin. * * Each executor: - * 1. Interpolates `{var}` / `{var.path}` / `{$User.*}` / `{NOW()}` tokens in - * `node.config` against the running flow's variable context — except in - * the `create_record` / `update_record` `fields` map, a value slot, where - * a CEL value envelope is evaluated to the value written and the `{…}` - * dialect is retired (#19939; {@link resolveFieldValues}). + * 1. Parses its config as authored against its contract, then evaluates its + * value slots in the running flow's scope — the `create_record` / + * `update_record` `fields` map ({@link resolveFieldValues}) and each + * operand of the `filter` ({@link resolveNodeFilter}): a CEL value + * envelope is evaluated, every other value is a literal, and the `{…}` + * template dialect is retired from both (#19939) — a query placeholder in + * a filter is the query engine's, handed on. * 2. Calls the resolved data engine via `ctx.getService('data')`. * 3. Writes the result back to the variable context under `outputVariable` * (or under `.id` / `.records` by default), so downstream - * nodes can reference fields like `{leadRecord.company}`. + * nodes can read fields like `leadRecord.company`. * * If no data engine is registered, executors degrade to a no-op success so * test environments without ObjectQL still complete the flow without errors. @@ -431,12 +451,13 @@ export function registerCrudNodes(engine: AutomationEngine, ctx: PluginContext): // string-keyed map (JSON-Schema `additionalProperties`), which the // designer renders with its flat keyValue editor; values stay // `true`-permissive because real metadata carries operator objects - // (e.g. `{"$ne": null}`) alongside `{var}` templates and literals. + // (e.g. `{"$ne": null}`) alongside CEL value envelopes, query + // placeholders and literals (#19939: each operand a value slot). configSchema: { type: 'object', properties: { objectName: { type: 'string', title: 'Object', xRef: { kind: 'object' } }, - filter: { type: 'object', additionalProperties: true, title: 'Filter', description: 'Field/value pairs to match (e.g. status → active). Operator values like {"$ne": null} are preserved.' }, + filter: { type: 'object', additionalProperties: true, title: 'Filter', description: 'Field/value pairs to match (e.g. status → active). Operator values like {"$ne": null} are preserved; each operand is a CEL value envelope evaluated before the query, a query placeholder such as {today}, or a literal.' }, // Declared in #4045: the executor has always passed this // straight into find/findOne as the projection, but no // form offered it — authorable only by hand until now. @@ -454,10 +475,10 @@ export function registerCrudNodes(engine: AutomationEngine, ctx: PluginContext): // `filters` → `filter` and `object` → `objectName` are handled at // load by the ADR-0087 D2 conversion layer ('flow-node-crud-filter-alias', // 'flow-node-crud-object-alias'), so the parse sees canonical keys - // (PD #12 fallbacks retired). Parsed BEFORE interpolation: every - // typed slot the contract declares beyond strings is read raw by - // this executor too (`limit` never honored a template), so a - // template in one was already dead config — now it is loud. + // (PD #12 fallbacks retired). Parsed as authored: every typed + // slot the contract declares beyond strings is read raw by this + // executor too (`limit` never honored a template), and a flow + // `{…}` token at a filter operand is refused here (#19939). const parsed = parseNodeConfig('get_record', node.id, GetRecordConfigSchema, node.config); if (!parsed.ok) return parsed.refusal; const cfg = parsed.config; @@ -465,7 +486,7 @@ export function registerCrudNodes(engine: AutomationEngine, ctx: PluginContext): if (!objectName) return refuseNode('get_record: objectName required'); const filterResult = resolveNodeFilter( - cfg.filter, variables, context, 'get_record', + engine, cfg.filter, variables, context, 'get_record', 'would have read rows the filter was written to exclude', ); if ('error' in filterResult) return refuseNode(filterResult.error); @@ -477,7 +498,7 @@ export function registerCrudNodes(engine: AutomationEngine, ctx: PluginContext): // [#21623] A filter that evaluates the stored-metadata family's // body or content hash is refused before the engine is asked, // with the data door's own code, under either run identity. It - // reads the interpolated filter, in the `where` slot both + // reads the resolved filter, in the `where` slot both // engine reads below hand it in. const familyRefusal = storedMetadataFilterRefusal('get_record', objectName, { where: filter }); if (familyRefusal) return familyRefusal; @@ -682,7 +703,7 @@ export function registerCrudNodes(engine: AutomationEngine, ctx: PluginContext): type: 'object', properties: { objectName: { type: 'string', title: 'Object', xRef: { kind: 'object' } }, - filter: { type: 'object', additionalProperties: true, title: 'Filter', description: 'Field/value pairs identifying the record(s) to update (e.g. id → {recordId}).' }, + filter: { type: 'object', additionalProperties: true, title: 'Filter', description: 'Field/value pairs identifying the record(s) to update. Each operand is a CEL value envelope evaluated before the query, a query placeholder such as {current_user_id}, or a literal.' }, fields: { type: 'object', additionalProperties: true, title: 'Field values', description: 'Field values to write.' }, // #5393 — the author's bulk DECLARATION. Off (default) // the engine accepts only a write that names one row by @@ -709,7 +730,7 @@ export function registerCrudNodes(engine: AutomationEngine, ctx: PluginContext): // `filters` → `filter` converted at load (ADR-0087 D2); read canonical. const filterResult = resolveNodeFilter( - cfg.filter, variables, context, 'update_record', + engine, cfg.filter, variables, context, 'update_record', 'would have matched — and overwritten — rows the filter was written to exclude', ); if ('error' in filterResult) return refuseNode(filterResult.error); @@ -782,7 +803,7 @@ export function registerCrudNodes(engine: AutomationEngine, ctx: PluginContext): type: 'object', properties: { objectName: { type: 'string', title: 'Object', xRef: { kind: 'object' } }, - filter: { type: 'object', additionalProperties: true, title: 'Filter', description: 'Field/value pairs identifying the record(s) to delete.' }, + filter: { type: 'object', additionalProperties: true, title: 'Filter', description: 'Field/value pairs identifying the record(s) to delete. Each operand is a CEL value envelope evaluated before the query, a query placeholder such as {current_user_id}, or a literal.' }, // #5393 — see update_record. Highest-stakes declaration // the flow language has: without it a predicate delete // is refused by the engine, with it every matched row @@ -810,7 +831,7 @@ export function registerCrudNodes(engine: AutomationEngine, ctx: PluginContext): // The highest-stakes of the three: an erased condition here is the // difference between deleting one row and emptying the object. const filterResult = resolveNodeFilter( - cfg.filter, variables, context, 'delete_record', + engine, cfg.filter, variables, context, 'delete_record', 'would have matched every remaining row and deleted it', ); if ('error' in filterResult) return refuseNode(filterResult.error); diff --git a/packages/services/service-automation/src/builtin/get-record-id-projection.integration.test.ts b/packages/services/service-automation/src/builtin/get-record-id-projection.integration.test.ts index f36815d228a..5ba3ad067df 100644 --- a/packages/services/service-automation/src/builtin/get-record-id-projection.integration.test.ts +++ b/packages/services/service-automation/src/builtin/get-record-id-projection.integration.test.ts @@ -158,7 +158,7 @@ describe('get_record answers the row id as output.id whatever its fields project }); } - it('a downstream update_record filtering on {read.id} updates the row the narrowed get_record read', async () => { + it('a downstream update_record filtering on read.id updates the row the narrowed get_record read', async () => { await ql.insert(OBJECT.name, { title: 'beta', status: 'open' }, { context: SYS }); const def = { name: 'pin_get_record_id_flow', label: 'pin_get_record_id_flow', type: 'autolaunched', runAs: 'system', @@ -166,7 +166,7 @@ describe('get_record answers the row id as output.id whatever its fields project nodes: [ { id: 'start', type: 'start', label: 'Start' }, { id: 'read', type: 'get_record', label: 'Read', config: { objectName: OBJECT.name, filter: { title: 'beta' }, fields: ['title'], outputVariable: 'rec' } }, - { id: 'close', type: 'update_record', label: 'Close', config: { objectName: OBJECT.name, filter: { id: '{read.id}' }, fields: { status: 'closed' } } }, + { id: 'close', type: 'update_record', label: 'Close', config: { objectName: OBJECT.name, filter: { id: { dialect: 'cel', source: 'read.id' } }, fields: { status: 'closed' } } }, { id: 'end', type: 'end', label: 'End' }, ], edges: [ diff --git a/packages/services/service-automation/src/builtin/get-record-stored-metadata-filter-refusal.integration.test.ts b/packages/services/service-automation/src/builtin/get-record-stored-metadata-filter-refusal.integration.test.ts index a07e0e7037e..5d8ee9859b5 100644 --- a/packages/services/service-automation/src/builtin/get-record-stored-metadata-filter-refusal.integration.test.ts +++ b/packages/services/service-automation/src/builtin/get-record-stored-metadata-filter-refusal.integration.test.ts @@ -310,18 +310,20 @@ describe("flow get_record refuses a filter that evaluates the stored-metadata fa }); for (const runAs of ['system', 'user'] as const) { - it(`runAs:'${runAs}': a filter whose body condition is built from a flow variable is judged after interpolation and refused`, async () => { + it(`runAs:'${runAs}': a filter whose body condition is built from a flow variable is judged after evaluation and refused`, async () => { // The authored filter names no family column: the condition list arrives - // through `{record.conds}`. Only the interpolated filter (the one the - // engine would run) shows that it reads the body. - const authored: Filter = { $and: '{record.conds}' }; + // through an envelope evaluating `record.conds`. Only the resolved filter + // (the one the engine would run) shows that it reads the body. + // [#19939, stage S6] The list was a `{record.conds}` token; a flow token + // at a filter operand is refused since S6, and the envelope evaluates. + const authored: Filter = { $and: { dialect: 'cel', source: 'record.conds' } }; for (const conds of [[{ metadata: { $contains: SENTINEL.slice(0, 12) } }], [{ metadata: { $contains: 'no-such-credential-prefix' } }]]) { const run = await runWatched(readThenCopyFlow(`built_${seq++}`, 'sys_metadata', runAs, 'one', authored), { record: { conds } }); expectRefusedRun(run, 'a variable-built body filter'); } // …and a body condition whose value is a flow variable. const run = await runWatched( - readThenCopyFlow(`valued_${seq++}`, 'sys_metadata', runAs, 'list', { metadata: { $contains: '{record.guess}' } }), + readThenCopyFlow(`valued_${seq++}`, 'sys_metadata', runAs, 'list', { metadata: { $contains: { dialect: 'cel', source: 'record.guess' } } }), { record: { guess: SENTINEL.slice(0, 12) } }, ); expectRefusedRun(run, 'a body filter with a variable value'); diff --git a/packages/services/service-automation/src/builtin/template.ts b/packages/services/service-automation/src/builtin/template.ts index 6b6bcdaceb5..c1f54b53fbc 100644 --- a/packages/services/service-automation/src/builtin/template.ts +++ b/packages/services/service-automation/src/builtin/template.ts @@ -30,16 +30,20 @@ * could never round a computed money value to its field's declared `scale`, * and nothing said why. * - * The interpolator walks objects, arrays, and primitives recursively so it - * can be applied wholesale to a node's `config.filter` block and its other - * value-like positions. The value slots (`fields.*`, `assignments.*`) no - * longer read this dialect (#19939): a `{…}` token there is refused before it - * gets here (`@objectstack/spec/automation`'s `flow-value-slot-template.ts`), - * each with its CEL spelling — the run user is the CEL scope's - * `current_user`, and a date macro is its CEL string form, `isoDate(today())` - * / `isoDatetime(now())` (the stdlib writes the bytes the date branch below - * writes). The `{$User.*}` and date branches below still answer the position - * that keeps this dialect, a `filter`. Nor do the TEXT slots — a notify + * The interpolator walks objects, arrays, and primitives recursively. The + * value slots (`fields.*`, `assignments.*`, and since #19939 every other + * computed value, each operand of a CRUD node's `filter` included) no longer + * read this dialect: a `{…}` token there is refused before it gets here + * (`@objectstack/spec/automation`'s `flow-value-slot-template.ts`), each with + * its CEL spelling — the run user is the CEL scope's `current_user`, and a + * date macro is its CEL string form, `isoDate(today())` / + * `isoDatetime(now())` (the stdlib writes the bytes the date branch below + * writes) — and a query placeholder in a filter (`{current_user_id}`, + * `{today}`) is the query engine's, handed to it untouched. What still reaches + * this interpolator is the value slots' residual call on a token-free literal + * (`value-slot-map.ts`, the identity on its text) and the legacy `assignment` + * shapes; both go with the single-brace resolver (ADR-0032 Sequencing 6). Nor + * do the TEXT slots — a notify * `title` / `message` / `actionUrl`, an http `url` and `headers` values, a * screen `title` / `description`, a refusing `end` node's `message` (#22110, * ADR-0032 D3; #19939): @@ -49,7 +53,6 @@ */ import type { AutomationContext } from '@objectstack/spec/contracts'; -import { isKnownFilterToken } from '@objectstack/spec/data'; import { nearestName, templateEngine } from '@objectstack/formula'; import { markGuardRefusal } from '../guard-refusal.js'; @@ -502,59 +505,3 @@ export function interpolate( return value; } -/** - * Interpolate a node's **filter** block (framework#3810). - * - * A filter value position is the one place where two `{…}` dialects meet: the - * flow template dialect (`{record.owner}`, `{$User.Id}`) and the filter - * placeholder dialect (`{current_user_id}`, `{current_year_start}` — declared - * in `@objectstack/spec` and resolved by `resolveFilterTokens()` in the query - * engine). Evaluation order decided the winner by accident: the flow - * interpolator ran first, found no flow variable named `current_year_start`, - * and returned `undefined` — so the placeholder never reached the engine that - * knows how to resolve it, and the condition silently vanished from the query. - * - * This hands that position back to the dialect that owns it. A whole-string - * token that (a) no flow variable resolves and (b) IS a recognised filter - * placeholder is passed through **verbatim** for the engine to expand. That is - * a transfer of ownership, not a lenient fallback: flow variables still win - * when both could match, and a token belonging to neither dialect is left to - * the caller's collapse guard to report. - * - * Only filter blocks use this. No other position reads the single-brace - * dialect any more (#19939: an `http` `url` and its headers are text slots, - * its `body` a value slot), so a bare `{current_year_start}` anywhere else is - * refused before a run starts rather than read as a query bound. - */ -export function interpolateFilter( - value: T, - variables: VariableMap, - context: AutomationContext, -): T { - if (typeof value === 'string') { - const single = /^\{([^{}]+)\}$/.exec(value); - if (single) { - const resolved = resolveToken(single[1], variables, context); - // Flow variables keep precedence — only an unresolved token is - // considered for hand-off, so a flow variable that happens to share - // a placeholder's name still shadows it (no silent reinterpretation - // of a template that works today). - if (resolved === undefined && isKnownFilterToken(single[1].trim())) { - return value; - } - return resolved as unknown as T; - } - return interpolateString(value, variables, context) as unknown as T; - } - if (Array.isArray(value)) { - return value.map(v => interpolateFilter(v, variables, context)) as unknown as T; - } - if (value && typeof value === 'object') { - const out: Record = {}; - for (const [k, v] of Object.entries(value as Record)) { - out[k] = interpolateFilter(v, variables, context); - } - return out as unknown as T; - } - return value; -} diff --git a/packages/services/service-automation/src/canonicalize-stored-flow.test.ts b/packages/services/service-automation/src/canonicalize-stored-flow.test.ts index 33e89e42c53..86ac0d2ac24 100644 --- a/packages/services/service-automation/src/canonicalize-stored-flow.test.ts +++ b/packages/services/service-automation/src/canonicalize-stored-flow.test.ts @@ -47,8 +47,8 @@ const legacyFlow = () => ({ // runs before the region canonicalization under test. body: { nodes: [ - { id: 'b1', type: 'update_record', label: 'Touch', config: { objectName: 'lead', filters: { id: '{item.id}' } } }, - { id: 'b2', type: 'update_record', label: 'Mark', config: { objectName: 'lead', filter: { id: '{item.id}' } } }, + { id: 'b1', type: 'update_record', label: 'Touch', config: { objectName: 'lead', filters: { id: { dialect: 'cel', source: 'item.id' } } } }, + { id: 'b2', type: 'update_record', label: 'Mark', config: { objectName: 'lead', filter: { id: { dialect: 'cel', source: 'item.id' } } } }, ], edges: [{ id: 'be1', source: 'b1', target: 'b2', condition: "status == 'x'" }], }, @@ -71,7 +71,7 @@ describe('canonicalizeStoredFlow — the storable shape (#4454)', () => { expect('filters' in s.nodes[1].config).toBe(false); // The conversion pass already reaches into regions — only the condition // envelope needs the schema's help. - expect(s.nodes[2].config.body.nodes[0].config.filter).toEqual({ id: '{item.id}' }); + expect(s.nodes[2].config.body.nodes[0].config.filter).toEqual({ id: { dialect: 'cel', source: 'item.id' } }); expect('filters' in s.nodes[2].config.body.nodes[0].config).toBe(false); }); diff --git a/packages/services/service-automation/src/engine.ts b/packages/services/service-automation/src/engine.ts index 0552b59de12..3c5268345d7 100644 --- a/packages/services/service-automation/src/engine.ts +++ b/packages/services/service-automation/src/engine.ts @@ -390,7 +390,7 @@ export interface SuspensionRelease { * connector rate-limited, the data engine rejected a write. The metadata is * fine and a later run could succeed. A declared `fault` edge routes it. * - `guard` — the METADATA is wrong, and a refuse-to-execute guard said so: - * interpolation erased a filter condition (#3810), a data node names no + * a filter operand evaluated to nothing (#3810), a data node names no * object, a run would execute unscoped (ADR-0049/#1888). Re-running changes * nothing, and the refusal IS the safety property. Not routable — it stays * fatal whether or not a `fault` edge exists. diff --git a/packages/services/service-automation/src/fault-edge-guard-containment.test.ts b/packages/services/service-automation/src/fault-edge-guard-containment.test.ts index 2c8aabab02f..3607e0073f9 100644 --- a/packages/services/service-automation/src/fault-edge-guard-containment.test.ts +++ b/packages/services/service-automation/src/fault-edge-guard-containment.test.ts @@ -63,9 +63,13 @@ describe('#3863 — a fault edge must not swallow a guard refusal', () => { }); /** - * The exact shape #3810 exists to stop, with a fault edge bolted on: - * `{record.ownr}` is a typo, so the only condition is erased and the filter - * would collapse to `{}` — every row in the object. + * The exact shape #3810 exists to stop, with a fault edge bolted on: the + * only condition's operand evaluates to nothing — the record carries no + * `manager`, so the guarded read answers `null` — and the filter would + * match every row in the object. ([#19939, stage S6] It was a + * `{record.ownr}` typo that interpolated to nothing; a flow token at a + * filter operand is refused by the contract parse now, and the guard holds + * for an operand envelope.) */ function guardedDeleteWithHandler() { return { @@ -79,7 +83,7 @@ describe('#3863 — a fault edge must not swallow a guard refusal', () => { id: 'op', type: 'delete_record' as any, label: 'Delete', - config: { objectName: 'deal', filter: { owner: '{record.ownr}' } }, + config: { objectName: 'deal', filter: { owner: { dialect: 'cel', source: 'has(record.manager) ? record.manager : null' } } }, }, { id: 'handler', type: 'script' as any, label: 'Handler', config: { function: 'noop' } }, { id: 'end', type: 'end' as const, label: 'End' }, diff --git a/packages/services/service-automation/src/guard-refusal-inventory.test.ts b/packages/services/service-automation/src/guard-refusal-inventory.test.ts index d6ae4325ab1..5d9bd08a70b 100644 --- a/packages/services/service-automation/src/guard-refusal-inventory.test.ts +++ b/packages/services/service-automation/src/guard-refusal-inventory.test.ts @@ -91,6 +91,9 @@ function flowWithHandler(name: string, node: Record) { * is refused at the build doors too, so its row registers the block whole and * removes it from the stored node before the run. */ +/** [#19939] A filter operand envelope that evaluates to `null` on the run below (its record has no `manager`). */ +const NOTHING = { dialect: 'cel', source: 'has(record.manager) ? record.manager : null' }; + const GUARDS: Array<{ name: string; why: string; node: Record; expect: string; strip?: string; stripBlock?: string }> = [ // Since #4277 a missing REQUIRED key is refused by the executor's contract // parse (parse-config.ts) before the hand-written guard runs, so those @@ -125,25 +128,30 @@ const GUARDS: Array<{ name: string; why: string; node: Record; strip: 'objectName', expect: 'does not satisfy the delete_record contract', }, + // [#19939, stage S6] RE-JUDGED IN PLACE: these three wrote the condition as + // a `{record.ownr}` token that interpolated to nothing. A flow token at a + // filter operand is refused by the contract parse since S6, and the #3810 + // guard holds for an operand ENVELOPE that evaluates to nothing — the run's + // record carries no `manager`, so the guarded read answers `null`. { - name: 'get_record whose filter lost a condition (#3810)', + name: 'get_record whose filter operand evaluated to nothing (#3810)', why: 'an erased condition WIDENS the query — the reason #3810 exists', - node: { type: 'get_record', config: { objectName: 'deal', filter: { owner: '{record.ownr}' } } }, + node: { type: 'get_record', config: { objectName: 'deal', filter: { owner: NOTHING } } }, expect: 'refusing to run', }, { - name: 'update_record whose filter lost a condition (#3810)', + name: 'update_record whose filter operand evaluated to nothing (#3810)', why: 'the widened query would overwrite rows the author excluded', node: { type: 'update_record', - config: { objectName: 'deal', filter: { owner: '{record.ownr}' }, fields: { stage: 'x' } }, + config: { objectName: 'deal', filter: { owner: NOTHING }, fields: { stage: 'x' } }, }, expect: 'refusing to run', }, { - name: 'delete_record whose filter lost a condition (#3810)', + name: 'delete_record whose filter operand evaluated to nothing (#3810)', why: 'the highest-stakes case — a collapsed filter empties the object', - node: { type: 'delete_record', config: { objectName: 'deal', filter: { owner: '{record.ownr}' } } }, + node: { type: 'delete_record', config: { objectName: 'deal', filter: { owner: NOTHING } } }, expect: 'refusing to run', }, { diff --git a/packages/services/service-automation/src/guard-refusal.ts b/packages/services/service-automation/src/guard-refusal.ts index 2ff3a610511..4e7c6c6c67b 100644 --- a/packages/services/service-automation/src/guard-refusal.ts +++ b/packages/services/service-automation/src/guard-refusal.ts @@ -43,7 +43,7 @@ export const GUARD_REFUSAL: unique symbol = Symbol.for( * Use it when BOTH hold: * 1. re-running the flow unchanged can never succeed, and * 2. the fix is to edit metadata (a missing required config key, a graph that - * recurses, a filter whose condition interpolation erased). + * recurses, a filter whose operand evaluated to nothing). * * If either is false the failure is a `runtime` one — the world did not * cooperate — and it must stay routable so authors can handle it. A connector diff --git a/packages/services/service-automation/src/record-lookup-expand.integration.test.ts b/packages/services/service-automation/src/record-lookup-expand.integration.test.ts index 9f5690cb02f..42d1eda31d1 100644 --- a/packages/services/service-automation/src/record-lookup-expand.integration.test.ts +++ b/packages/services/service-automation/src/record-lookup-expand.integration.test.ts @@ -54,7 +54,7 @@ function expandFlow(name: string, runAs: 'system' | 'user', expand: string[], fi variables: [{ name: 'noteId', type: 'text', isInput: true }], nodes: [ { id: 'start', type: 'start', label: 'Start', config: { objectName: 'lead', triggerType: 'record-after-create', expand } }, - { id: 'up', type: 'update_record', label: 'Up', config: { objectName: 'audit', filter: { id: '{noteId}' }, fields } }, + { id: 'up', type: 'update_record', label: 'Up', config: { objectName: 'audit', filter: { id: { dialect: 'cel', source: 'noteId' } }, fields } }, { id: 'end', type: 'end', label: 'End' }, ], edges: [{ id: 'e1', source: 'start', target: 'up' }, { id: 'e2', source: 'up', target: 'end' }], diff --git a/packages/services/service-automation/src/run-summary.test.ts b/packages/services/service-automation/src/run-summary.test.ts index 2b333f3940f..8a43b1edfa3 100644 --- a/packages/services/service-automation/src/run-summary.test.ts +++ b/packages/services/service-automation/src/run-summary.test.ts @@ -351,7 +351,7 @@ function idempotentSweepFlow(name: string, shape: 'healthy' | 'dead_gate') { const lookup = { id: 'find_existing', type: 'get_record', label: 'Already handled?', // No `limit` ⇒ findOne ⇒ `selected: 1` when it finds one, `0` when not. - config: { objectName: 'nudge', filter: { deal: '{row.id}' }, outputVariable: 'existing' }, + config: { objectName: 'nudge', filter: { deal: { dialect: 'cel', source: 'row.id' } }, outputVariable: 'existing' }, }; const gate = { id: 'gate', type: 'decision', label: 'Gate' }; const nudge = { id: 'nudge', type: 'create_record', label: 'Nudge', config: { objectName: 'nudge', fields: { note: 'x' } } }; diff --git a/packages/services/service-automation/src/runas-grant-resolution.integration.test.ts b/packages/services/service-automation/src/runas-grant-resolution.integration.test.ts index 2d30596d215..cd9a48fc11d 100644 --- a/packages/services/service-automation/src/runas-grant-resolution.integration.test.ts +++ b/packages/services/service-automation/src/runas-grant-resolution.integration.test.ts @@ -55,7 +55,7 @@ function updateFlow(name: string, runAs: 'system' | 'user') { variables: [{ name: 'noteId', type: 'text', isInput: true }], nodes: [ { id: 'start', type: 'start', label: 'Start' }, - { id: 'up', type: 'update_record', label: 'Update', config: { objectName: 'runas_thing', filter: { id: '{noteId}' }, fields: { status: 'x' } } }, + { id: 'up', type: 'update_record', label: 'Update', config: { objectName: 'runas_thing', filter: { id: { dialect: 'cel', source: 'noteId' } }, fields: { status: 'x' } } }, { id: 'end', type: 'end', label: 'End' }, ], edges: [{ id: 'e1', source: 'start', target: 'up' }, { id: 'e2', source: 'up', target: 'end' }], diff --git a/packages/spec/api-surface/automation.json b/packages/spec/api-surface/automation.json index 7c418ef9f2b..9b21a98491b 100644 --- a/packages/spec/api-surface/automation.json +++ b/packages/spec/api-surface/automation.json @@ -125,6 +125,8 @@ "FlowEdge (type)", "FlowEdgeParsed (type)", "FlowEdgeSchema (const)", + "FlowFilterOperand (interface)", + "FlowFilterOperandStringKind (type)", "FlowFunctionCallable (type)", "FlowFunctionDeclaration (type)", "FlowFunctionDeclarationParsed (type)", @@ -279,6 +281,7 @@ "approverTypeIsOrgScoped (function)", "builtinNodeConfigKeysJudged (function)", "canonicalApproverType (function)", + "classifyFlowFilterOperandString (function)", "collectFlowGraphs (function)", "defineActionDescriptor (function)", "defineFlow (function)", @@ -286,6 +289,7 @@ "describeMissingScheduleOrganization (function)", "exportConstructsToBpmn (function)", "findRegionEntry (function)", + "flowFilterOperands (function)", "flowForm (const)", "flowNodeConfigRefusals (function)", "flowNodeTextSlotSources (function)", diff --git a/packages/spec/dropped-refinements.baseline.json b/packages/spec/dropped-refinements.baseline.json index 55687c29af4..56a53976f73 100644 --- a/packages/spec/dropped-refinements.baseline.json +++ b/packages/spec/dropped-refinements.baseline.json @@ -2,8 +2,8 @@ "description": "Shrink-only ledger of every PUBLISHED JSON Schema that is STILL WIDER than the Zod type it was generated from, because a rule written as `.refine()` reaches the runtime and not the file (#18670). `z.toJSONSchema()` has no arm for a `custom` check: a plain record, the same record with a `.refine()`, and the same record with an ABORTING `.refine()` all project byte-identically (measured on zod 4.4.3, the version packages/spec resolves). So a document one of these files ACCEPTS can still be refused at parse time, and an author -- or an AI -- validating against packages/spec/json-schema/** finds out a release later. Each `sites` path is a position under that schema at which a refinement is dropped; the same paths are written onto the artifact itself as `x-dropped-refinements`. Item 2 closed the first patterns: a refinement DECLARED through the closed list in src/shared/refinement-projection.ts is emitted into the published file, reads `projected` rather than `dropped`, and its row LEAVES this ledger in the same PR -- which is why the ledger shrinks and never grows on a repair. Every refinement outside that closed list stays here, and adding an arm to the list is a public-contract decision, not a refactor. Hand-edited on purpose and with no `gen:` script: a generator would let a new gap be admitted by running a command instead of by a decision, which is the silence this ledger exists to end. Adding, removing or moving a site fails packages/spec/scripts/build-schemas.ts until the line moves with it, and the failure prints the corrected entry in full. ⛔ Do not delete or weaken a refinement to shorten this file -- the runtime rule is correct; it is the projection that is silent, and the remedy is to teach the closed list a NAMED pattern, never to drop the rule.", "measured": { "zod": "4.4.3", - "publishedSchemasWithDroppedRefinements": 232, - "droppedRefinementSites": 752, + "publishedSchemasWithDroppedRefinements": 234, + "droppedRefinementSites": 755, "refinementSitesThatDidProject": 369, "refinementSitesWithNoJsonFormToCompare": 0 }, @@ -497,6 +497,11 @@ "" ] }, + "automation/DeleteRecordConfig": { + "sites": [ + "filter" + ] + }, "automation/EndConfig": { "sites": [ "" @@ -531,6 +536,11 @@ "definition.nodes.element.in.waitEventConfig" ] }, + "automation/GetRecordConfig": { + "sites": [ + "filter" + ] + }, "automation/HttpConfig": { "sites": [ "", @@ -601,7 +611,8 @@ }, "automation/UpdateRecordConfig": { "sites": [ - "fields.valueType" + "fields.valueType", + "filter" ] }, "data/AggregationNode": { diff --git a/packages/spec/export-origins/automation.json b/packages/spec/export-origins/automation.json index 34eed20778f..5626398aa72 100644 --- a/packages/spec/export-origins/automation.json +++ b/packages/spec/export-origins/automation.json @@ -121,6 +121,8 @@ "FlowEdge": "src/automation/flow.zod.ts#FlowEdge (type)", "FlowEdgeParsed": "src/automation/flow.zod.ts#FlowEdgeParsed (type)", "FlowEdgeSchema": "src/automation/flow.zod.ts#FlowEdgeSchema (const)", + "FlowFilterOperand": "src/automation/flow-node-expression-paths.ts#FlowFilterOperand (interface)", + "FlowFilterOperandStringKind": "src/automation/flow-value-slot-template.ts#FlowFilterOperandStringKind (type)", "FlowFunctionCallable": "src/automation/flow-function.zod.ts#FlowFunctionCallable (type)", "FlowFunctionDeclaration": "src/automation/flow-function.zod.ts#FlowFunctionDeclaration (type)", "FlowFunctionDeclarationParsed": "src/automation/flow-function.zod.ts#FlowFunctionDeclarationParsed (type)", @@ -273,6 +275,7 @@ "approverTypeIsOrgScoped": "src/automation/approval.zod.ts#approverTypeIsOrgScoped (function)", "builtinNodeConfigKeysJudged": "src/automation/flow-node-config-refusals.ts#builtinNodeConfigKeysJudged (function)", "canonicalApproverType": "src/automation/approval.zod.ts#canonicalApproverType (function)", + "classifyFlowFilterOperandString": "src/automation/flow-value-slot-template.ts#classifyFlowFilterOperandString (function)", "collectFlowGraphs": "src/automation/control-flow.zod.ts#collectFlowGraphs (function)", "defineActionDescriptor": "src/automation/node-executor.zod.ts#defineActionDescriptor (function)", "defineFlow": "src/automation/flow.zod.ts#defineFlow (function)", @@ -280,6 +283,7 @@ "describeMissingScheduleOrganization": "src/automation/schedule-organization.zod.ts#describeMissingScheduleOrganization (function)", "exportConstructsToBpmn": "src/automation/bpmn-mapping.ts#exportConstructsToBpmn (function)", "findRegionEntry": "src/automation/control-flow.zod.ts#findRegionEntry (function)", + "flowFilterOperands": "src/automation/flow-node-expression-paths.ts#flowFilterOperands (function)", "flowForm": "src/automation/flow.form.ts#flowForm (const)", "flowNodeConfigRefusals": "src/automation/flow-node-config-refusals.ts#flowNodeConfigRefusals (function)", "flowNodeTextSlotSources": "src/automation/flow-text-slot-template.ts#flowNodeTextSlotSources (function)", diff --git a/packages/spec/src/automation/builtin-node-config.test.ts b/packages/spec/src/automation/builtin-node-config.test.ts index 68f05150551..1423cbc9674 100644 --- a/packages/spec/src/automation/builtin-node-config.test.ts +++ b/packages/spec/src/automation/builtin-node-config.test.ts @@ -43,6 +43,7 @@ import { getSchemalessNodeConfigJsonSchemas, } from './schemaless-node-config.zod.js'; import { VALUE_SLOT_TEMPLATE_REFUSAL } from './flow-value-slot-template.js'; +import { flowNodeConfigRefusals } from './flow-node-config-refusals.js'; interface Parseable { safeParse(v: unknown): { success: boolean; error?: { issues: ReadonlyArray<{ code: string; message: string }> } } } @@ -583,8 +584,11 @@ describe('assignment value contract — a CEL envelope beside literals; the `{to // #19938 — the CRUD write map joined through the same channel; #19939 — // the `map` node's per-item `input` map, then the `screen`'s prefill and // record id, then the `notify` node's computed values, then the `http` - // node's body. - expect(Object.keys(LEDGER_DECLARED_NODE_CONFIG_SCHEMAS)).toEqual(['assignment', 'create_record', 'update_record', 'map', 'screen', 'notify', 'http']); + // node's body, then (stage S6) the CRUD filter operands — `update_record` + // was here already, so `get_record` and `delete_record` join. + expect(Object.keys(LEDGER_DECLARED_NODE_CONFIG_SCHEMAS)).toEqual([ + 'assignment', 'create_record', 'update_record', 'map', 'screen', 'notify', 'http', 'get_record', 'delete_record', + ]); expect(Object.keys(SCHEMALESS_NODE_CONFIG_SCHEMAS).sort()).toEqual(['decision', 'script', 'subflow']); const projected = getSchemalessNodeConfigJsonSchemas().assignment as { properties?: Record }> }; @@ -844,7 +848,7 @@ describe('CRUD `fields` value contract — the CEL value envelope beside literal const PRICE_ENVELOPE = { dialect: 'cel', source: 'round(price * 100) / 100.0' }; const configs = [ ['create_record', CreateRecordConfigSchema, (fields: unknown) => ({ objectName: 'quote', fields })], - ['update_record', UpdateRecordConfigSchema, (fields: unknown) => ({ objectName: 'quote', filter: { id: '{quoteId}' }, fields })], + ['update_record', UpdateRecordConfigSchema, (fields: unknown) => ({ objectName: 'quote', filter: { id: { dialect: 'cel', source: 'quoteId' } }, fields })], ] as const; it.each(configs)('%s: accepts a valid CEL value envelope as a field value, stored verbatim', (_type, schema, wrap) => { @@ -910,8 +914,100 @@ describe('CRUD `fields` value contract — the CEL value envelope beside literal const projected = getSchemalessNodeConfigJsonSchemas()[type] as { properties?: Record }> }; expect(projected.properties?.fields?.additionalProperties?.xExpression).toBe('value'); - // `update_record.filter` is a match map, not a value slot — no marker. + // [#19939, stage S6] RE-JUDGED IN PLACE: `update_record.filter` carried no + // marker as a match map. Its OPERANDS are value slots since S6, declared + // once on `filter` itself (the ledger entry's `filterOperands` makes the + // walks emit each operand) — never on its map value, a second slot. expect(projected.properties?.filter?.additionalProperties?.xExpression).toBeUndefined(); + expect((projected.properties?.filter as { xExpression?: string } | undefined)?.xExpression) + .toBe(type === 'update_record' ? 'value' : undefined); + }); +}); + +/** + * [#19939, stage S6] A `get_record` / `update_record` / `delete_record` node's + * `filter` is a match condition whose OPERANDS are value slots (Fork R17 → A): + * a CEL value envelope evaluated before the query, a query placeholder the + * query engine resolves, or a literal. A flow `{…}` token at an operand is + * refused, located at the operand, with its CEL spelling. + */ +describe('CRUD `filter` operands — value slots; the query placeholders stay (#19939, S6)', () => { + const ENV = { dialect: 'cel', source: 'record.id' }; + const contracts = [ + ['get_record', GetRecordConfigSchema], + ['update_record', UpdateRecordConfigSchema], + ['delete_record', DeleteRecordConfigSchema], + ] as const; + const issuesOf = (schema: Parseable, filter: unknown) => { + const result = schema.safeParse({ objectName: 'deal', filter }); + return result.success ? [] : (result.error!.issues as ReadonlyArray<{ code: string; path: PropertyKey[]; message: string }>); + }; + + it.each(contracts.flatMap(([type, schema]) => [ + [type, 'a field\'s value', schema, { id: '{record.id}' }, 'filter.id'], + [type, 'an operator\'s comparand', schema, { amount: { $gt: '{record.amount}' } }, 'filter.amount.$gt'], + [type, 'an `$in` element', schema, { id: { $in: ['r1', '{record.id}'] } }, 'filter.id.$in.1'], + [type, 'a `$or` member\'s value', schema, { $or: [{ status: 'open' }, { owner: '{record.owner}' }] }, 'filter.$or.1.owner'], + [type, 'a nested relation\'s value', schema, { account: { industry: '{record.industry}' } }, 'filter.account.industry'], + ] as const))('%s REFUSES a flow token at %s — code `custom`, at the operand, with its envelope', (_type, _where, schema, filter, path) => { + const issues = issuesOf(schema, filter); + expect(issues.map((i) => [i.code, ['filter', ...i.path.slice(1)].join('.')])).toEqual([['custom', path]]); + expect(issues[0]!.message.startsWith(VALUE_SLOT_TEMPLATE_REFUSAL)).toBe(true); + expect(issues[0]!.message).toContain('an operand envelope is evaluated before the query'); + expect(issues[0]!.message).toContain('An operand that evaluates to `null` refuses the node'); + }); + + it.each(contracts)('%s ACCEPTS an envelope at every operand position, stored verbatim', (_type, schema) => { + const filter = { + id: ENV, amount: { $gt: ENV }, ref: { $in: [ENV, 'r2'] }, $or: [{ owner: ENV }], account: { industry: ENV }, + }; + const result = schema.safeParse({ objectName: 'deal', filter }); + expect(result.success).toBe(true); + expect((result.data as { filter: unknown }).filter).toEqual(filter); + }); + + it.each(contracts)('%s ACCEPTS a literal and hands a query placeholder on untouched, at any depth', (_type, schema) => { + const filter = { + status: 'open', + owner_id: '{current_user_id}', + org: '${current_org_id}', + due: { $lt: '{today}', $gte: '{30_days_ago}' }, + $and: [{ closed_at: { $between: ['{last_month_start}', '{last_month_end}'] } }], + note: { $contains: 'quarterly review' }, + n: { $gte: 3 }, flag: true, gone: null, + }; + const result = schema.safeParse({ objectName: 'deal', filter }); + expect(result.success).toBe(true); + expect((result.data as { filter: unknown }).filter).toEqual(filter); + }); + + it.each(contracts)('%s REFUSES a malformed envelope at an operand — led by the slot-neutral sentence, at its source', (_type, schema) => { + const issues = issuesOf(schema, { owner: { $eq: { dialect: 'cel', source: ' ' } } }); + expect(issues.map((i) => [i.code, ['filter', ...i.path.slice(1)].join('.')])).toEqual([['custom', 'filter.owner.$eq.source']]); + expect(issues[0]!.message.startsWith(VALUE_ENVELOPE_REFUSAL)).toBe(true); + }); + + it.each(contracts)('%s: the build doors\' value arm leaves an operand\'s envelope and token to their own judges — one finding, never two', (type) => { + const extra = type === 'update_record' ? { fields: { stage: 'x' } } : {}; + // The envelope pass (`registerFlow`, `objectstack validate`) refuses the + // malformed envelope, and the value-slot judge the token; the contract arm + // must not report either a second time. What holds it: the `filter` ledger + // rows make every issue under the filter one inside a value slot, which the + // contract arm leaves to those judges. + expect(flowNodeConfigRefusals(type, { objectName: 'deal', filter: { owner: { $eq: { dialect: 'cel', source: ' ' } } }, ...extra })).toEqual([]); + expect(flowNodeConfigRefusals(type, { objectName: 'deal', filter: { owner: '{record.owner}' }, ...extra })).toEqual([]); + // CONTROL: a filter that is not a map is still the contract's to refuse. + expect(flowNodeConfigRefusals(type, { objectName: 'deal', filter: 'open', ...extra }).map((r) => [r.code, r.path])) + .toEqual([['node-config-refused-by-contract', 'filter']]); + }); + + it.each(contracts)('%s declares the operands to the expression ledger on `filter`, once', (type, schema) => { + const direct = z.toJSONSchema(schema, { target: 'draft-2020-12', io: 'input', unrepresentable: 'any' }) as + { properties?: Record }> }; + expect(direct.properties?.filter?.xExpression).toBe('value'); + expect(direct.properties?.filter?.additionalProperties?.xExpression).toBeUndefined(); + const projected = getSchemalessNodeConfigJsonSchemas()[type] as typeof direct; + expect(projected.properties?.filter?.xExpression).toBe('value'); }); }); diff --git a/packages/spec/src/automation/builtin-node-config.zod.ts b/packages/spec/src/automation/builtin-node-config.zod.ts index 1116b9ce6a1..7914986b7da 100644 --- a/packages/spec/src/automation/builtin-node-config.zod.ts +++ b/packages/spec/src/automation/builtin-node-config.zod.ts @@ -37,7 +37,8 @@ * where values interpolate), so `{token}` templates pass and resolve at the * executor's existing interpolation points. The value slots are the exception * since #19939: the `{token}` dialect is retired there (the "value slots" - * section below). So are the text slots since #22110: a screen `title` / + * section below), each operand of the CRUD `filter` included, where a query + * placeholder stays the query engine's. So are the text slots since #22110: a screen `title` / * `description` and an `end` `message` render `{{ }}` holes, and a `{token}` * there is refused (`flow-text-slot-template.ts`). * @@ -77,7 +78,11 @@ * refused there since #19939 — and those maps are the expression ledger's * `value`-role slots, as are, since #19939, the three maps a node hands to a * callee, a `screen`'s `defaults.*`, field `defaultValue` and `recordId`, and - * the list a `loop` and a `map` iterate (`FlowCollectionSlotSchema`). + * the list a `loop` and a `map` iterate (`FlowCollectionSlotSchema`), and each + * operand of the CRUD `filter` — a field's value, an operator's comparand, each + * element of a list comparand — which a CEL value envelope computes before the + * query while a query placeholder (`{current_user_id}`, `{today}`) stays the + * query engine's (the "CRUD quartet" section below). * * Deliberately absent: * - `decision` / `script` / `subflow` / `wait` / `connector_action` — the @@ -95,7 +100,7 @@ import { EvaluatedExpressionSchema } from '../shared/expression.zod'; import { lazySchema } from '../shared/lazy-schema'; import { strictObject } from '../shared/strict-object'; import { refuseCatchallProtoKey, refuseRecordProtoKey } from '../shared/record-proto-key-guard'; -import { isExpressionEnvelopeShaped } from './flow-node-expression-paths'; +import { flowFilterOperands, isExpressionEnvelopeShaped } from './flow-node-expression-paths'; // [#19939] The retirement of the `{…}` template dialect from value slots — the // one judge, composed into every value slot's contract below rather than // re-spelled here. @@ -181,9 +186,9 @@ const CRUD_ALIAS_GUIDANCE = { */ const CRUD_RECORD_ID_GUIDANCE = 'CRUD nodes address rows through `filter`, never through a `recordId` key — no executor has ever read one. ' - + "Write the id as a filter VALUE: `filter: { id: '{record.id}' }`, which is the shape the node's own descriptor " - + 'documents. This matters most on `delete_record`: a config whose only "constraint" is an unread key is a ' - + 'match-everything delete, the hazard.'; + + "Write the id as a filter operand: `filter: { id: { dialect: 'cel', source: 'record.id' } }`, a CEL value " + + 'envelope evaluated before the query, which is the shape the node\'s own descriptor documents. This matters most ' + + 'on `delete_record`: a config whose only "constraint" is an unread key is a match-everything delete, the hazard.'; /** * `fieldValues` — the AI-authoring dialect that never had a runtime reader. @@ -265,8 +270,9 @@ const CRUD_BULK_INTENT_GUIDANCE = { * `subflow.input.*`, `map.input.*`, `script.inputs.*`, a `screen`'s * `defaults.*`, `fields[].defaultValue` and `recordId`, a `loop`'s and a * `map`'s `collection`, a `notify` node's `recipients`, `templateData.*` - * and `sourceId`, and an `http` node's `body`), so an author (or an agent - * reading the issue) learns the rule before the detail. + * and `sourceId`, an `http` node's `body`, and each operand of a CRUD + * node's `filter`), so an author (or an agent reading the issue) learns the + * rule before the detail. * * Slot-neutral on purpose (#19938): the refusal's LOCATION names the slot * (`config.fields.`, `config.assignments.`). A sentence that @@ -427,7 +433,9 @@ function celValueSlotSchema(description: string) { * `notify` node's `templateData` map (`NotifyConfigSchema`; its `recipients` * and `sourceId` take the same rule with a string literal, a list of them for * `recipients`), and so does an `http` node's `body` (`HttpConfigSchema`, one - * value: the whole payload), and it is + * value: the whole payload), and so does each operand of a CRUD node's + * `filter` (its own contract, {@link flowFilterSlotSchema}: the same rule, with + * the query placeholders handed on), and it is * what a consumer judging ANY value-role slot applies (`AutomationEngine`'s * `valueEnvelopeRefusals`, the lint's `checkDeclaredValue`). The `assignment` * map's {@link AssignmentValueSchema} is the same rule under a @@ -533,14 +541,76 @@ export type FlowCollectionSlotParsed = z.infer; // ─── CRUD quartet ──────────────────────────────────────────────────── +/** + * A `get_record` / `update_record` / `delete_record` node's `filter` (#19939, + * Fork R17 → A; ADR-0032 Decision 2: "Computed value (`dueDate`, filter + * values)" serialises as a CEL envelope) — the match condition, a + * `FilterCondition` tree, whose OPERANDS are value slots + * (`flowFilterOperands`: a field's value, an operator's comparand, each + * element of a list comparand). Its keys, operators, combinators and + * nested-relation conditions are its structure. Each operand is: + * + * - a **CEL value envelope** — {@link AssignmentExpressionValueSchema}, + * evaluated by the executor in the run's scope BEFORE the query; one that + * evaluates to `null` refuses the node rather than widening the query + * (#3810), as a `{token}` that resolved to nothing did. A malformed envelope + * leads with {@link VALUE_ENVELOPE_REFUSAL}, as in every value slot; + * - a **query placeholder** (`{current_user_id}`, `{today}`, `{30_days_ago}`) + * — the query engine's own vocabulary, which it resolves; handed to the + * query untouched (`classifyFlowFilterOperandString`, the one classifier); + * - any other **literal**, handed to the query as written — and a string in + * it that carries a `{…}` token of the retired flow template dialect is + * refused with its CEL spelling ({@link valueSlotTemplateRefusals} with + * `filterOperand`). + * + * The record type, and every rule a filter's SHAPE answers to — an operator's + * comparand, an empty combinator — stay the query engine's, judged where the + * query runs, as before. Built per contract for its description, eagerly, for + * the reason {@link celValueSlotSchema} gives; the `xExpression: 'value'` + * marker declares the operands to the expression ledger's reconciliation + * ratchet, whose `filter` entries carry `filterOperands`. + */ +function flowFilterSlotSchema(description: string) { + return z.record(z.string(), z.unknown()) + .superRefine((filter, ctx) => { + for (const operand of flowFilterOperands(filter)) { + if (!isExpressionEnvelopeShaped(operand.value)) { + for (const refusal of valueSlotTemplateRefusals(operand.value, { filterOperand: true })) { + ctx.addIssue({ code: 'custom', path: [...operand.path, ...refusal.path], message: refusal.message }); + } + continue; + } + const result = AssignmentExpressionValueSchema.safeParse(operand.value); + if (result.success) continue; + for (const issue of result.error.issues) { + const where = issue.path.length > 0 ? `\`${issue.path.map(String).join('.')}\`: ` : ''; + ctx.addIssue({ + code: 'custom', + path: [...operand.path, ...issue.path], + message: `${VALUE_ENVELOPE_REFUSAL} ${where}${issue.message}`, + }); + } + } + }) + .meta({ description, xExpression: 'value' }); +} + +/** What every CRUD `filter` description ends with — the operand rule. */ +const FILTER_OPERAND_DESCRIPTION = + 'each operand — a field\'s value, an operator\'s comparand ({"$ne": null}), a list comparand\'s element — is a CEL ' + + 'value envelope `{ dialect: \'cel\', source }` evaluated before the query (one that evaluates to null refuses the ' + + 'node), a query placeholder such as `{current_user_id}` or `{today}` the query engine resolves, or a literal; a ' + + 'flow `{…}` template token is refused'; + /** * `get_record` node config — what the executor reads. * * `objectName` is execute-time required (the step is refused without it). - * `filter` values may carry operator objects (`{"$ne": null}`) and `{token}` - * templates; a template that resolves to nothing REFUSES the node rather than - * widening the query (#3810). `limit > 1` selects `find` (a `records` list); - * otherwise `findOne` (a single `record`). + * `filter` values may carry operator objects (`{"$ne": null}`), and each + * operand is a CEL value envelope, a query placeholder or a literal + * ({@link flowFilterSlotSchema}); an envelope that evaluates to nothing + * REFUSES the node rather than widening the query (#3810). `limit > 1` selects + * `find` (a `records` list); otherwise `findOne` (a single `record`). */ export const GetRecordConfigSchema = lazySchema(() => strictObject({ surface: 'this get_record node config', @@ -549,9 +619,8 @@ export const GetRecordConfigSchema = lazySchema(() => strictObject({ }, { /** Object to query (execute-time required). */ objectName: z.string().describe('Object to query'), - /** Field/value pairs to match; operator objects and `{token}` templates are legal values. */ - filter: z.record(z.string(), z.unknown()).optional() - .describe('Field/value pairs to match (operator values like {"$ne": null} are preserved)'), + /** Field/value pairs to match; each operand an envelope, a query placeholder or a literal ({@link flowFilterSlotSchema}). */ + filter: flowFilterSlotSchema(`Field/value pairs to match: ${FILTER_OPERAND_DESCRIPTION}`).optional(), /** Field projection — only these fields are read (default: all). */ fields: z.array(z.string()).optional() .describe('Field projection — only these fields are read (default: all)'), @@ -613,9 +682,12 @@ export const UpdateRecordConfigSchema = lazySchema(() => strictObject({ }, { /** Object to update (execute-time required). */ objectName: z.string().describe('Object to update'), - /** Field/value pairs identifying the record(s) to update; an erased template condition refuses the node (#3810). */ - filter: z.record(z.string(), z.unknown()).optional() - .describe('Field/value pairs identifying the record(s) to update'), + /** + * Field/value pairs identifying the record(s) to update; each operand an + * envelope, a query placeholder or a literal ({@link flowFilterSlotSchema}), + * and an operand that evaluates to nothing refuses the node (#3810). + */ + filter: flowFilterSlotSchema(`Field/value pairs identifying the record(s) to update: ${FILTER_OPERAND_DESCRIPTION}`).optional(), /** * Field values to write — a `value`-role slot of the expression ledger * (`update_record.fields.*`, #19938): each value is a CEL value envelope or @@ -639,9 +711,9 @@ export const UpdateRecordConfigSchema = lazySchema(() => strictObject({ * count. Same word the engine has always used for the concept * (`EngineUpdateOptions.multi`) — one concept, one name (PD #12). * - * It does not weaken the #3810 erased-condition guard: a node whose authored - * filter condition interpolated to nothing is still refused before any write, - * `multi` or not. What it DOES make reachable is the deliberate whole-object + * It does not weaken the #3810 erased-condition guard: a node whose filter + * operand evaluated to nothing is still refused before any write, `multi` or + * not. What it DOES make reachable is the deliberate whole-object * write — `multi: true` with no `filter` at all is every row, by declaration. */ multi: z.boolean().optional() @@ -664,9 +736,12 @@ export const DeleteRecordConfigSchema = lazySchema(() => strictObject({ }, { /** Object to delete from (execute-time required). */ objectName: z.string().describe('Object to delete from'), - /** Field/value pairs identifying the record(s) to delete. */ - filter: z.record(z.string(), z.unknown()).optional() - .describe('Field/value pairs identifying the record(s) to delete'), + /** + * Field/value pairs identifying the record(s) to delete; each operand an + * envelope, a query placeholder or a literal ({@link flowFilterSlotSchema}), + * and an operand that evaluates to nothing refuses the node (#3810). + */ + filter: flowFilterSlotSchema(`Field/value pairs identifying the record(s) to delete: ${FILTER_OPERAND_DESCRIPTION}`).optional(), /** * Declare BULK intent — this node may delete EVERY row `filter` matches. * @@ -683,7 +758,7 @@ export const DeleteRecordConfigSchema = lazySchema(() => strictObject({ * * The highest-stakes key on this shape, which is why it is a DECLARATION and * not an inference from the filter's shape: the #3810 erased-condition guard - * still refuses a node whose authored condition interpolated to nothing, but + * still refuses a node whose filter operand evaluated to nothing, but * `multi: true` with no `filter` at all is the whole object, by declaration. * Write the constraint you mean. */ diff --git a/packages/spec/src/automation/flow-builtin-node-config-keys.test.ts b/packages/spec/src/automation/flow-builtin-node-config-keys.test.ts index ec555895534..dbbb58061ed 100644 --- a/packages/spec/src/automation/flow-builtin-node-config-keys.test.ts +++ b/packages/spec/src/automation/flow-builtin-node-config-keys.test.ts @@ -52,8 +52,8 @@ const NOTIFY: Config = { recipients: { dialect: 'cel', source: 'record.owner' }, const VALID: Readonly> = { get_record: { objectName: 'task', filter: { status: 'open' }, limit: 5, outputVariable: 'rows' }, create_record: { objectName: 'task', fields: { title: 'X' }, outputVariable: 'created' }, - update_record: { objectName: 'task', filter: { id: '{recordId}' }, fields: { title: 'X' } }, - delete_record: { objectName: 'task', filter: { id: '{recordId}' } }, + update_record: { objectName: 'task', filter: { id: { dialect: 'cel', source: 'recordId' } }, fields: { title: 'X' } }, + delete_record: { objectName: 'task', filter: { id: { dialect: 'cel', source: 'recordId' } } }, notify: NOTIFY, http: { url: 'https://example.test/hook', method: 'POST', headers: { 'X-Any': 'y' } }, screen: { title: 'T', fields: [{ name: 'a', type: 'select', options: [{ label: 'A', value: 'a' }] }] }, @@ -211,7 +211,7 @@ describe('the remainder: every other builtin, refused at save', () => { }); it('a pre-conversion alias is converted first where the door converts, and refused at a direct parse', () => { - const config = { objectName: 'task', filters: { id: '{recordId}' }, fields: { title: 'X' } }; + const config = { objectName: 'task', filters: { id: { dialect: 'cel', source: 'recordId' } }, fields: { title: 'X' } }; expect(issuesOf(flowWith('update_record', config)).map((i) => i.path)).toEqual(['nodes.1.config.filters']); const stack = { manifest: { id: 'com.example.alias', name: 'alias', version: '1.0.0', type: 'app', namespace: 'als' }, diff --git a/packages/spec/src/automation/flow-builtin-node-config-values.test.ts b/packages/spec/src/automation/flow-builtin-node-config-values.test.ts index bb4378c548f..63d47b61b1c 100644 --- a/packages/spec/src/automation/flow-builtin-node-config-values.test.ts +++ b/packages/spec/src/automation/flow-builtin-node-config-values.test.ts @@ -44,8 +44,8 @@ const REGION = region('inner'); const VALID: Readonly> = { get_record: { objectName: 'account', filter: { status: 'open' }, fields: ['name'], limit: 10, outputVariable: 'rows' }, create_record: { objectName: 'task', fields: { title: 'X' }, outputVariable: 'taskId' }, - update_record: { objectName: 'task', filter: { id: '{record.id}' }, fields: { status: 'done' }, multi: false }, - delete_record: { objectName: 'task', filter: { id: '{record.id}' }, multi: false }, + update_record: { objectName: 'task', filter: { id: { dialect: 'cel', source: 'record.id' } }, fields: { status: 'done' }, multi: false }, + delete_record: { objectName: 'task', filter: { id: { dialect: 'cel', source: 'record.id' } }, multi: false }, notify: { recipients: ['u1'], title: 'Hello', severity: 'info', channels: ['inbox'] }, http: { url: 'https://example.test/hook', method: 'POST', headers: { 'X-Kind': 'probe' }, durable: false, timeoutMs: 5000 }, screen: { diff --git a/packages/spec/src/automation/flow-node-config-required.test.ts b/packages/spec/src/automation/flow-node-config-required.test.ts index 9ca529f422c..8d8fe83ed9e 100644 --- a/packages/spec/src/automation/flow-node-config-required.test.ts +++ b/packages/spec/src/automation/flow-node-config-required.test.ts @@ -85,8 +85,8 @@ const SCREEN: Config = { const CENSUS: Array<{ type: string; config: Config; key: string }> = [ { type: 'get_record', config: { objectName: 'account', outputVariable: 'rows' }, key: 'objectName' }, { type: 'create_record', config: { objectName: 'account', fields: { name: 'Acme' } }, key: 'objectName' }, - { type: 'update_record', config: { objectName: 'account', filter: { id: '{record.id}' }, fields: { name: 'Acme' } }, key: 'objectName' }, - { type: 'delete_record', config: { objectName: 'account', filter: { id: '{record.id}' } }, key: 'objectName' }, + { type: 'update_record', config: { objectName: 'account', filter: { id: { dialect: 'cel', source: 'record.id' } }, fields: { name: 'Acme' } }, key: 'objectName' }, + { type: 'delete_record', config: { objectName: 'account', filter: { id: { dialect: 'cel', source: 'record.id' } } }, key: 'objectName' }, { type: 'notify', config: { recipients: [{ dialect: 'cel', source: 'record.owner' }], title: 'Hi' }, key: 'recipients' }, { type: 'http', config: { url: 'https://example.com/hook' }, key: 'url' }, { type: 'screen', config: SCREEN, key: 'fields[0].name' }, diff --git a/packages/spec/src/automation/flow-node-expression-paths.test.ts b/packages/spec/src/automation/flow-node-expression-paths.test.ts index ae5ded62be9..33e97f30d95 100644 --- a/packages/spec/src/automation/flow-node-expression-paths.test.ts +++ b/packages/spec/src/automation/flow-node-expression-paths.test.ts @@ -18,6 +18,7 @@ import { describe, expect, it } from 'vitest'; import { FLOW_NODE_EXPRESSION_PATHS, + flowFilterOperands, isExpressionEnvelopeShaped, resolveFlowNodeExpressions, resolveFlowNodeValueSlots, @@ -140,9 +141,12 @@ describe('FLOW_NODE_EXPRESSION_PATHS — the assignment value entry', () => { describe('FLOW_NODE_EXPRESSION_PATHS — the CRUD `fields.*` value entries', () => { const PRICE_ENVELOPE = { dialect: 'cel', source: 'round(price * 100) / 100.0' }; - it.each(['create_record', 'update_record'] as const)('declares exactly one slot for `%s`: `fields.*`, role `value`', (nodeType) => { + // [#19939, stage S6] RE-JUDGED IN PLACE: `update_record` declared `fields.*` + // alone; since S6 its `filter` operands are a value slot too (the filter + // entries below), so `fields.*` is its first entry, not its only one. + it.each(['create_record', 'update_record'] as const)('declares `fields.*` for `%s`, role `value`, and beside it only a filter-operand entry', (nodeType) => { const entries = FLOW_NODE_EXPRESSION_PATHS.filter((e) => e.nodeType === nodeType); - expect(entries).toHaveLength(1); + expect(entries.map((e) => e.path)).toEqual(nodeType === 'create_record' ? ['fields.*'] : ['fields.*', 'filter']); expect(entries[0]!.path).toBe('fields.*'); expect(entries[0]!.role).toBe('value'); expect(entries[0]!.label).toBe(`${nodeType} field value`); @@ -175,13 +179,21 @@ describe('FLOW_NODE_EXPRESSION_PATHS — the CRUD `fields.*` value entries', () })).toEqual([]); }); - it('`update_record.filter` is NOT a value slot — an envelope-shaped filter value resolves nothing', () => { - expect(resolveFlowNodeExpressions('update_record', { filter: { total: PRICE_ENVELOPE } })).toEqual([]); + // [#19939, stage S6] RE-JUDGED IN PLACE, both: these pinned a CRUD + // `filter` as no value slot, an envelope-shaped filter value resolving + // nothing. Each filter OPERAND is a value slot since S6, so the envelope + // operand resolves at its own path; `get_record`'s `fields` projection stays + // no slot. + it('`update_record.filter`: an envelope operand resolves at its own path', () => { + expect(resolveFlowNodeExpressions('update_record', { filter: { total: PRICE_ENVELOPE } }) + .map((f) => [f.path, f.entry.label])).toEqual([['filter.total', 'update_record filter operand']]); }); - it('`get_record` / `delete_record` declare no value slot — their `fields` / `filter` resolve nothing', () => { - expect(resolveFlowNodeExpressions('get_record', { fields: ['id'], filter: { total: PRICE_ENVELOPE } })).toEqual([]); - expect(resolveFlowNodeExpressions('delete_record', { filter: { total: PRICE_ENVELOPE } })).toEqual([]); + it('`get_record` / `delete_record`: an envelope filter operand resolves; the `fields` projection does not', () => { + expect(resolveFlowNodeExpressions('get_record', { fields: ['id'], filter: { total: PRICE_ENVELOPE } }) + .map((f) => [f.path, f.entry.label])).toEqual([['filter.total', 'get_record filter operand']]); + expect(resolveFlowNodeExpressions('delete_record', { filter: { total: PRICE_ENVELOPE } }) + .map((f) => [f.path, f.entry.label])).toEqual([['filter.total', 'delete_record filter operand']]); }); it('a `fields` that is not a plain object resolves nothing rather than throwing', () => { @@ -433,7 +445,11 @@ describe('resolveFlowNodeValueSlots — every authored value of a `value` slot, expect(resolveFlowNodeValueSlots('screen', { fields: [{ visibleWhen: 'a == 1' }] })).toEqual([]); // [#19939] A body-less `loop` (the legacy flat-graph form): its `collection` is read by no value slot. expect(resolveFlowNodeValueSlots('loop', { collection: '{rows}' })).toEqual([]); - expect(resolveFlowNodeValueSlots('update_record', { filter: { id: '{x}' } })).toEqual([]); + // [#19939, stage S6] RE-JUDGED IN PLACE: an `update_record` filter operand + // was no value slot and resolved nothing; it is one since S6 (the filter + // operand block below pins what it resolves), so the undeclared map here + // is `create_record`'s `filter`, a key that node does not declare. + expect(resolveFlowNodeValueSlots('create_record', { filter: { id: '{x}' } })).toEqual([]); expect(resolveFlowNodeValueSlots('assignment', { digest: '{x}' })).toEqual([]); expect(resolveFlowNodeValueSlots('assignment', { assignments: [{ variable: 'd', value: '{x}' }] })).toEqual([]); expect(resolveFlowNodeValueSlots('create_record', null)).toEqual([]); @@ -446,6 +462,79 @@ describe('resolveFlowNodeValueSlots — every authored value of a `value` slot, }); }); +/** + * [#19939, stage S6] The operands of a CRUD node's `filter` are value slots + * (Fork R17 → A): `flowFilterOperands` reads the filter the way the filter + * grammar does, and both walks emit each OPERAND — never the filter's keys, + * operators, combinators or nested-relation conditions — at its own path. + */ +describe('a CRUD `filter`: each operand is a value slot of its own (#19939, S6)', () => { + const ENV = { dialect: 'cel', source: 'record.id' }; + + it.each(['get_record', 'update_record', 'delete_record'] as const)('%s declares one filter entry, `filterOperands`, role `value`', (nodeType) => { + const entry = FLOW_NODE_EXPRESSION_PATHS.find((e) => e.nodeType === nodeType && e.path === 'filter'); + expect(entry).toEqual({ + nodeType, path: 'filter', role: 'value', label: `${nodeType} filter operand`, filterOperands: true, + }); + }); + + it('every operand shape: a field\'s value, an operator\'s comparand, a list comparand\'s elements, combinators, a nested relation', () => { + const filter = { + owner: ENV, + status: 'open', + amount: { $gt: ENV, $lte: 100 }, + id: { $in: [ENV, 'r2'] }, + due: { $between: ['{yesterday}', ENV] }, + $or: [{ stage: ENV }, { stage: { $ne: null } }], + $not: { kind: 'x' }, + account: { industry: ENV }, + ref: { $eq: { $field: 'other' } }, + }; + expect(flowFilterOperands(filter).map((o) => [o.path, o.value])).toEqual([ + [['owner'], ENV], + [['status'], 'open'], + [['amount', '$gt'], ENV], + [['amount', '$lte'], 100], + [['id', '$in', 0], ENV], + [['id', '$in', 1], 'r2'], + [['due', '$between', 0], '{yesterday}'], + [['due', '$between', 1], ENV], + [['$or', 0, 'stage'], ENV], + [['$or', 1, 'stage', '$ne'], null], + [['$not', 'kind'], 'x'], + [['account', 'industry'], ENV], + [['ref', '$eq'], { $field: 'other' }], + ]); + }); + + it('an envelope is ONE operand, which may evaluate to a list; the top-level filter is never one', () => { + expect(flowFilterOperands({ id: { $in: ENV } }).map((o) => o.path)).toEqual([['id', '$in']]); + // A filter written as an envelope is a condition on fields named `dialect` and `source`. + expect(flowFilterOperands(ENV).map((o) => [o.path, o.value])).toEqual([[['dialect'], 'cel'], [['source'], 'record.id']]); + // A related field named `dialect` is matched through an operator map, never read as an envelope. + expect(flowFilterOperands({ rel: { dialect: { $eq: 'cel' } } }).map((o) => [o.path, o.value])) + .toEqual([[['rel', 'dialect', '$eq'], 'cel']]); + }); + + it('a filter that is not a plain object has no operand; a malformed combinator member is handed over as one', () => { + expect(flowFilterOperands('{x}')).toEqual([]); + expect(flowFilterOperands(null)).toEqual([]); + expect(flowFilterOperands([ENV])).toEqual([]); + expect(flowFilterOperands({ $and: ['{x}'] }).map((o) => [o.path, o.value])).toEqual([[['$and', 0], '{x}']]); + }); + + it('both walks emit the operands at their ledger paths — every value, and the envelope subset', () => { + const config = { objectName: 'task', filter: { owner: ENV, id: { $in: ['{recordId}', ENV] }, $or: [{ s: 'x' }] } }; + expect(resolveFlowNodeValueSlots('delete_record', config).map((f) => [f.path, f.value])).toEqual([ + ['filter.owner', ENV], + ['filter.id.$in[0]', '{recordId}'], + ['filter.id.$in[1]', ENV], + ['filter.$or[0].s', 'x'], + ]); + expect(resolveFlowNodeExpressions('delete_record', config).map((f) => f.path)).toEqual(['filter.owner', 'filter.id.$in[1]']); + }); +}); + describe('isExpressionEnvelopeShaped — the recognizer a value slot discriminates on', () => { it('is a plain object with a string `dialect`, and nothing else', () => { expect(isExpressionEnvelopeShaped({ dialect: 'cel', source: '1' })).toBe(true); @@ -473,7 +562,8 @@ describe('every entry older than the value role resolves byte-identically (the r // moved `loop.collection` and `map.collection` from the retired // `flow-template` role to `value` IN PLACE (`screen.fields[].visibleWhen` // keeps its `predicate` row beside the screen's value rows). Stage S4 - // appended the three `notify` value rows, and stage S5 the `http` body's. + // appended the three `notify` value rows, stage S5 the `http` body's, and + // stage S6 the three CRUD `filter` operand rows. expect(FLOW_NODE_EXPRESSION_PATHS.map(byKey)).toEqual([ 'screen.fields[].visibleWhen (predicate)', 'decision.conditions[].expression (predicate)', @@ -492,6 +582,9 @@ describe('every entry older than the value role resolves byte-identically (the r 'notify.templateData.* (value)', 'notify.sourceId (value)', 'http.body (value)', + 'get_record.filter (value)', + 'update_record.filter (value)', + 'delete_record.filter (value)', ]); }); diff --git a/packages/spec/src/automation/flow-node-expression-paths.ts b/packages/spec/src/automation/flow-node-expression-paths.ts index fee2ec0577f..54829972778 100644 --- a/packages/spec/src/automation/flow-node-expression-paths.ts +++ b/packages/spec/src/automation/flow-node-expression-paths.ts @@ -160,6 +160,15 @@ export type FlowNodeExpressionRole = * (`flow-text-slot-template.ts`), and its `method`, `durable`, `timeoutMs` * and `signingSecret` are literals, read as written, whose `{…}` token the * contract refuses (`HttpConfigSchema`). + * + * And for each OPERAND of a `get_record`, `update_record` or `delete_record` + * node's `filter` — a field's value, an operator's comparand, each element + * of a list comparand ({@link FlowNodeExpressionPath.filterOperands}) — + * evaluated before the query. The filter's keys, operators, combinators and + * nested-relation conditions are its structure, never a value. A query + * placeholder (`{current_user_id}`, `{today}`) at an operand is the query + * engine's own vocabulary, handed to it untouched (ADR-0032 puts the + * query-filter surface out of its scope). */ | 'value'; @@ -230,6 +239,21 @@ export interface FlowNodeExpressionPath { * reads only arrays and so would never reach a single value. */ readonly eachElement?: true; + /** + * [#19939] The slot is a query FILTER — a `FilterCondition` tree — and each + * of its OPERANDS is a value slot of its own: today the `filter` of + * `get_record`, `update_record` and `delete_record`. Both walks + * ({@link resolveFlowNodeExpressions}, {@link resolveFlowNodeValueSlots}) + * emit each operand at its own path ({@link flowFilterOperands}: + * `filter.owner`, `filter.amount.$gt`, `filter.id.$in[0]`, + * `filter.$or[1].status`), so an envelope operand is validated and + * evaluated where it sits and a `{…}` token in a string operand is refused + * with the envelope that replaces it there, while the filter's keys, + * operators, combinators and nested-relation conditions stay its structure. + * ⛔ Not a `*` path segment: a filter is a tree, and its operands sit at + * every depth. + */ + readonly filterOperands?: true; } /** @@ -243,9 +267,7 @@ export interface FlowNodeExpressionPath { * are *structural* predicate surfaces on every node and edge rather than * declared config properties, and both validators already walk them. * - * Also deliberately absent: the config values that still INTERPOLATE - * `{token}` templates — the positions the #19939 retirement has not reached - * yet (a CRUD `filter` value) — and the TEXT slots, which render `{{ }}` + * Also deliberately absent: the TEXT slots, which render `{{ }}` * holes since #22110 (a notify `title` / `message` and, since #19939, its * `actionUrl` and an http node's `url` and `headers` values, a screen * `title` / `description`, an `end` `message`; their one judge and list is @@ -306,6 +328,15 @@ export interface FlowNodeExpressionPath { * (a CEL map literal builds an object), declared through the spec Zod channel * (`HttpConfigSchema` in `LEDGER_DECLARED_NODE_CONFIG_SCHEMAS`; the * descriptor offers `body` with no marker). + * + * So are, since #19939, the operands of a `get_record`, `update_record` and + * `delete_record` node's `filter` — each declared ONCE, on the `filter` key, + * through the spec Zod channel (`GetRecordConfigSchema`, + * `UpdateRecordConfigSchema`, `DeleteRecordConfigSchema` in + * `LEDGER_DECLARED_NODE_CONFIG_SCHEMAS`; the descriptors offer `filter` as an + * open map with no marker). The entry carries + * {@link FlowNodeExpressionPath.filterOperands}: the walks emit each operand + * at its own path, never the filter itself. */ export const FLOW_NODE_EXPRESSION_PATHS: readonly FlowNodeExpressionPath[] = [ { @@ -395,9 +426,9 @@ export const FLOW_NODE_EXPRESSION_PATHS: readonly FlowNodeExpressionPath[] = [ label: 'create_record field value', }, { - // The same map on the update node (`UpdateRecordConfigSchema`); its - // `filter` map is NOT a value slot — a filter is a match condition, not a - // value to compute. + // The same map on the update node (`UpdateRecordConfigSchema`). Its + // `filter` is a match condition, not a value to compute — but each of its + // OPERANDS is one (the `filter` entries below). nodeType: 'update_record', path: 'fields.*', role: 'value', @@ -523,8 +554,151 @@ export const FLOW_NODE_EXPRESSION_PATHS: readonly FlowNodeExpressionPath[] = [ role: 'value', label: 'http body', }, + { + // [#19939] The operands of a CRUD node's `filter` (Fork R17 → A; ADR-0032 + // Decision 2: "Computed value (`dueDate`, filter values)" serialises as a + // CEL envelope). Each operand — a field's value, an operator's comparand, + // each element of a list comparand — is a CEL value envelope the executor + // evaluates in the run's scope BEFORE the query, or a literal handed to + // the query as written; a query placeholder (`{current_user_id}`, + // `{today}`) is a literal of the query engine's own vocabulary, which it + // resolves. An operand that evaluates to `null` refuses the node: an + // absent condition widens the query (#3810). Declared on `filter` through + // the spec Zod channel (`GetRecordConfigSchema`, carried to the ratchet by + // `LEDGER_DECLARED_NODE_CONFIG_SCHEMAS`); the descriptor offers `filter` + // as an open map with no marker. + nodeType: 'get_record', + path: 'filter', + role: 'value', + label: 'get_record filter operand', + filterOperands: true, + }, + { + // The same operands on the update node (`UpdateRecordConfigSchema`). + nodeType: 'update_record', + path: 'filter', + role: 'value', + label: 'update_record filter operand', + filterOperands: true, + }, + { + // And on the delete node (`DeleteRecordConfigSchema`), where an absent + // condition costs the most. + nodeType: 'delete_record', + path: 'filter', + role: 'value', + label: 'delete_record filter operand', + filterOperands: true, + }, ]; +/** One operand of a flow node's query filter ({@link flowFilterOperands}). */ +export interface FlowFilterOperand { + /** + * Where the operand sits inside the filter — `['owner']`, + * `['amount', '$gt']`, `['id', '$in', 0]`, `['$or', 1, 'status']`, + * `['account', 'industry']` (a nested-relation condition). + */ + readonly path: readonly (string | number)[]; + /** The authored operand: an envelope, a string, any other literal. */ + readonly value: unknown; +} + +/** A plain object — not an array, not `null`, not a class instance such as a `Date`. */ +function isPlainFilterObject(value: unknown): value is Record { + if (value === null || typeof value !== 'object' || Array.isArray(value)) return false; + const proto = Object.getPrototypeOf(value); + return proto === Object.prototype || proto === null; +} + +/** + * **Every OPERAND of a flow node's query filter** (#19939) — the positions a + * value sits at, as opposed to the filter's structure — read the way the + * filter grammar reads a `FilterCondition` (`data/filter.zod.ts`): + * + * - `$and` / `$or` hold a list of conditions and `$not` one condition: each + * is walked as a condition, never an operand; + * - any other key names a field, and its value is + * - an **envelope** (an object naming a string `dialect`, + * {@link isExpressionEnvelopeShaped}) — an operand, the implicit + * equality's computed comparand; + * - an **operator map** (an object with a `$` key) — each entry's value is + * an operand (`$gt`, `$in`, `$between`, …); + * - any other **object** — a nested-relation condition (the filter + * grammar's form 4), walked as a condition; + * - anything else — an operand, the implicit equality's literal; + * - an operand that is a LIST (`$in: [a, b]`, `$between: [lo, hi]`) is a + * list of operands, each at its own index; one that is an envelope is ONE + * operand, which may evaluate to a list. + * + * So an envelope is told apart from every literal object a filter holds by + * position and shape: at an operator's comparand or a list element a plain + * object is no literal the query takes (a `{ $field }` reference names a `$` + * key, not a `dialect`), and at a field's value an object without a `$` key is + * a nested-relation condition — one whose first level names a string + * `dialect` is read as an envelope, so a related field named `dialect` is + * matched as `{ dialect: { $eq: '…' } }`, an operator map. The top-level + * filter is a condition, never an operand. + * + * Whatever sits where a combinator wants a condition and is none — a value + * that is no plain object (`$and: 'x'`, `$and: [42]`), or an envelope + * (`$not: { dialect: 'cel', … }`) — is handed over as an operand, so every + * string the filter carries is reached and an envelope anywhere is evaluated + * (what it evaluates to is the query engine's to judge, as any operand's is). + * Cycle-safe: a flow built in code may hold a self-reference. Pure — no + * judgement about what an operand says. + */ +export function flowFilterOperands(filter: unknown): FlowFilterOperand[] { + const out: FlowFilterOperand[] = []; + const seen = new Set(); + const operand = (value: unknown, path: readonly (string | number)[]): void => { + if (Array.isArray(value)) { + value.forEach((element, index) => out.push({ path: [...path, index], value: element })); + return; + } + out.push({ path, value }); + }; + const conditionKeys = (node: Record, path: readonly (string | number)[]): void => { + if (seen.has(node)) return; + seen.add(node); + for (const [key, value] of Object.entries(node)) { + const here = [...path, key]; + if ((key === '$and' || key === '$or') && Array.isArray(value)) { + value.forEach((member, index) => condition(member, [...here, index])); + continue; + } + if (key === '$not') { + condition(value, here); + continue; + } + if (isPlainFilterObject(value) && !isExpressionEnvelopeShaped(value)) { + if (Object.keys(value).some((k) => k.startsWith('$'))) { + for (const [op, comparand] of Object.entries(value)) operand(comparand, [...here, op]); + } else { + conditionKeys(value, here); + } + continue; + } + operand(value, here); + } + }; + // A combinator's member: a condition, unless it is no plain object or is an + // envelope — then it is handed over as an operand. + const condition = (node: unknown, path: readonly (string | number)[]): void => { + if (!isPlainFilterObject(node) || isExpressionEnvelopeShaped(node)) operand(node, path); + else conditionKeys(node, path); + }; + if (isPlainFilterObject(filter)) conditionKeys(filter, []); + return out; +} + +/** An operand's path appended to a ledger path, in the ledger's spelling (`filter.id.$in[0]`). */ +function operandPath(prefix: string, path: readonly (string | number)[]): string { + let out = prefix; + for (const segment of path) out += typeof segment === 'number' ? `[${segment}]` : (out ? `.${segment}` : segment); + return out; +} + /** One resolved expression value found in a node's config. */ export interface ResolvedFlowNodeExpression { /** The ledger entry that matched. */ @@ -609,7 +783,9 @@ export function isExpressionEnvelopeShaped(value: unknown): value is { dialect: * An entry with {@link FlowNodeExpressionPath.onlyWithKey} is skipped on a * config that does not carry that key (#19939): its executor never reads the * slot there. An entry with {@link FlowNodeExpressionPath.eachElement} reads - * each element of an array as a value of its own, at its index (#19939). + * each element of an array as a value of its own, at its index (#19939). An + * entry with {@link FlowNodeExpressionPath.filterOperands} reads each operand + * of the filter as a value of its own, at its path (#19939). */ export function resolveFlowNodeExpressions( nodeType: string, @@ -651,7 +827,9 @@ export function resolveFlowNodeExpressions( * are value slots. * An absent (`undefined`) value is skipped; everything else is handed over * verbatim, with its concrete path — on an {@link FlowNodeExpressionPath.eachElement} - * entry holding an array, each element at its own index. + * entry holding an array, each element at its own index, and on a + * {@link FlowNodeExpressionPath.filterOperands} entry each operand of the + * filter at its own path. * * Pure path resolution, like its sibling — no judgement about what a value * says. @@ -672,12 +850,20 @@ export function resolveFlowNodeValueSlots(nodeType: string, config: unknown): Re * The emitter a walk hands each value the entry's path reaches — `emit` itself, * or for an {@link FlowNodeExpressionPath.eachElement} entry one that hands * over each element of an array at its own index (`recipients[1]`) and any - * other value at the slot's own path. + * other value at the slot's own path, or for a + * {@link FlowNodeExpressionPath.filterOperands} entry one that hands over each + * operand of the filter at its own path (`filter.amount.$gt`) and never the + * filter itself. */ function slotEmitter( entry: FlowNodeExpressionPath, emit: (path: string, value: unknown) => void, ): (path: string, value: unknown) => void { + if (entry.filterOperands) { + return (path, value) => { + for (const found of flowFilterOperands(value)) emit(operandPath(path, found.path), found.value); + }; + } if (!entry.eachElement) return emit; return (path, value) => { if (!Array.isArray(value)) { diff --git a/packages/spec/src/automation/flow-region-pause-and-end.test.ts b/packages/spec/src/automation/flow-region-pause-and-end.test.ts index 00d006e974a..6b6f580aca7 100644 --- a/packages/spec/src/automation/flow-region-pause-and-end.test.ts +++ b/packages/spec/src/automation/flow-region-pause-and-end.test.ts @@ -249,7 +249,7 @@ describe('the rule does NOT over-reach', () => { // judges only this rule — a config left out is refused by its own (#20316). const WHOLE: Record> = { create_record: { objectName: 'task' }, - update_record: { objectName: 'task', filter: { id: '{row.id}' } }, + update_record: { objectName: 'task', filter: { id: { dialect: 'cel', source: 'row.id' } } }, get_record: { objectName: 'task' }, http: { url: 'https://example.com/hook' }, notify: { recipients: [{ dialect: 'cel', source: 'row.owner' }], title: 'Row processed' }, diff --git a/packages/spec/src/automation/flow-template-token.ts b/packages/spec/src/automation/flow-template-token.ts index e54e81dba20..0284999c6b6 100644 --- a/packages/spec/src/automation/flow-template-token.ts +++ b/packages/spec/src/automation/flow-template-token.ts @@ -230,9 +230,8 @@ export function runUserPathNeverResolved(text: string, write: (path: string) => `\`${text}\` never resolved in any shipped run: it read a user object no run carries, so the template ` + 'wrote nothing here. `current_user` carries only what the run holds — `id`, ' + '`positions`, `organizationId`, `isPlatformAdmin`. For the user\'s email or name, read the user record by ' - + `\`current_user.id\`: compute the id into a variable with an \`assignment\` node (\`assignments: { uid: ` - + `{ dialect: 'cel', source: '${CEL_RUN_USER_ID}' } }\`), read the record with a \`get_record\` node ` - + '(`objectName: \'sys_user\'`, `filter: { id: \'{uid}\' }`, `outputVariable: \'me\'`), and write ' + + '`current_user.id` with a `get_record` node (`objectName: \'sys_user\'`, `filter: { id: { dialect: ' + + `'cel', source: '${CEL_RUN_USER_ID}' } }\`, \`outputVariable: 'me'\`), and write ` + `${write('me.email')} or ${write('me.name')}.` ); } diff --git a/packages/spec/src/automation/flow-value-slot-template.test.ts b/packages/spec/src/automation/flow-value-slot-template.test.ts index fbb65d90b21..605c23497ac 100644 --- a/packages/spec/src/automation/flow-value-slot-template.test.ts +++ b/packages/spec/src/automation/flow-value-slot-template.test.ts @@ -55,6 +55,7 @@ import { import { FLOW_NODE_EXPRESSION_PATHS } from './flow-node-expression-paths'; import { VALUE_SLOT_TEMPLATE_REFUSAL, + classifyFlowFilterOperandString, flowNodeValueTemplateRefusals, valueSlotTemplateRefusals, } from './flow-value-slot-template'; @@ -75,10 +76,14 @@ function configAtEntry(path: string, value: unknown): Record { /** * {@link configAtEntry}, on a config the entry's slot is READ on: an entry with * `onlyWithKey` (`loop.collection`, read only with a `body` — #19939) gets that - * key too. + * key too, and an entry with `filterOperands` (a CRUD `filter`, #19939 stage + * S6) holds the value at an operand — a field's value — never as the filter. */ -function configForEntry(entry: { readonly path: string; readonly onlyWithKey?: string }, value: unknown): Record { - const config = configAtEntry(entry.path, value); +function configForEntry( + entry: { readonly path: string; readonly onlyWithKey?: string; readonly filterOperands?: true }, + value: unknown, +): Record { + const config = entry.filterOperands ? configAtEntry(`${entry.path}.*`, value) : configAtEntry(entry.path, value); return entry.onlyWithKey === undefined ? config : { ...config, [entry.onlyWithKey]: { nodes: [], edges: [] } }; } @@ -396,7 +401,11 @@ describe('the run user — `{$User.*}` is refused, with the ruled remedies', () const message = refusalOf(token); expect(message).toContain(`\`${token}\` never resolved in any shipped run`); expect(message).toContain('`current_user` carries only what the run holds — `id`, `positions`, `organizationId`, `isPlatformAdmin`'); - expect(message).toContain("assignments: { uid: { dialect: 'cel', source: 'current_user.id' } }"); + // [#19939, stage S6] The read names the run user's id in the filter + // operand itself — the `{uid}` token it used to compute first is refused + // there now. + expect(message).toContain("filter: { id: { dialect: 'cel', source: 'current_user.id' } }"); + expect(message).not.toContain('{uid}'); expect(message).toContain("objectName: 'sys_user'"); expect(message).toContain("{ dialect: 'cel', source: 'me.email' }"); expect(message).not.toContain("Write `"); @@ -516,7 +525,11 @@ describe('every value-slot contract refuses the same set, at the value', () => { }); describe('`flowNodeValueTemplateRefusals` — every value position of a node, located for a door', () => { - it('create_record / update_record `fields`, nested values included; a `filter` value is not a value slot', () => { + // [#19939, stage S6] RE-JUDGED IN PLACE: this pinned an `update_record` + // `filter` value as no value slot. Each filter operand is one since S6, so + // the update node's `{recordId}` operand is refused after its `fields`; the + // create node declares no `filter`, so its one stays unjudged. + it('create_record / update_record `fields`, nested values included; a filter operand on the update node only', () => { for (const nodeType of ['create_record', 'update_record']) { const refusals = flowNodeValueTemplateRefusals(nodeType, { objectName: 'task', @@ -527,6 +540,7 @@ describe('`flowNodeValueTemplateRefusals` — every value position of a node, lo ['fields.owner', `${nodeType} field value`], ['fields.tags[0]', `${nodeType} field value`], ['fields.due', `${nodeType} field value`], + ...(nodeType === 'update_record' ? [['filter.id', 'update_record filter operand']] : []), ]); } }); @@ -884,6 +898,93 @@ describe('a notify node\'s value slots: located per position, a recipient\'s ref }); }); +/** + * [#19939, stage S6] Each operand of a CRUD node's `filter` is a value slot + * (Fork R17 → A). Two `{…}` vocabularies met there; the query placeholders + * stay — the query engine resolves them — and the flow token is refused. ONE + * classifier draws the line, and the remedy names no guard answering `null`: + * an operand that evaluates to `null` refuses the node (#3810). + */ +describe('a CRUD filter operand: the query placeholders pass, a flow token is refused (#19939, S6)', () => { + const filterRefusalOf = (value: unknown): string => { + const refusals = valueSlotTemplateRefusals(value, { filterOperand: true }); + expect(refusals, `exactly one refusal for ${JSON.stringify(value)}`).toHaveLength(1); + expect(refusals[0]!.message.startsWith(VALUE_SLOT_TEMPLATE_REFUSAL)).toBe(true); + return refusals[0]!.message; + }; + + it.each([ + ['{current_user_id}', 'placeholder'], + ['{current_org_id}', 'placeholder'], + ['{today}', 'placeholder'], + ['${today}', 'placeholder'], + ['{30_days_ago}', 'placeholder'], + ['{current_year_start}', 'placeholder'], + ['{record.id}', 'flow-token'], + ['{recordId}', 'flow-token'], + ['{record_id}', 'flow-token'], + ['{TODAY()}', 'flow-token'], + ['{$User.Id}', 'flow-token'], + ['{ today }', 'flow-token'], + ['due {today}', 'flow-token'], + ['open', 'literal'], + ['', 'literal'], + ] as const)('classifies %s as %s — the one classifier', (value, kind) => { + expect(classifyFlowFilterOperandString(value)).toBe(kind); + }); + + it('a query placeholder is not judged, at any depth of a literal operand', () => { + expect(valueSlotTemplateRefusals('{current_user_id}', { filterOperand: true })).toEqual([]); + expect(valueSlotTemplateRefusals(['{today}', '{yesterday}'], { filterOperand: true })).toEqual([]); + // CONTROL: outside a filter operand the same string is a refused token. + expect(valueSlotTemplateRefusals('{today}')).toHaveLength(1); + }); + + it('a path: its envelope, and what `null` does at a filter operand — never a guard answering `null`', () => { + const message = filterRefusalOf('{record.id}'); + expect(message).toContain("Write `{record.id}` as { dialect: 'cel', source: 'record.id' }: an operand envelope is evaluated before the query."); + expect(message).toContain('An operand that evaluates to `null` refuses the node, as a token that resolved to nothing did'); + expect(message).toContain('route around the node with a `decision`'); + expect(message).not.toContain('has(vars.'); + expect(message).not.toContain('the guarded form writes `null`'); + }); + + it('the run user\'s id: `current_user.id`, and a decision where the flow can run without a user', () => { + const message = filterRefusalOf('{$User.Id}'); + expect(message).toContain("Write `{$User.Id}` as { dialect: 'cel', source: 'current_user.id' }"); + expect(message).toContain('skip the node there with a `decision` on `current_user != null`'); + expect(message).not.toContain('current_user != null ? current_user.id : null'); + }); + + it('an expression keeps its decimal divisor; a date macro is its CEL string form', () => { + expect(filterRefusalOf('{round(x * 100) / 100}')).toContain("source: 'round(x * 100) / 100.0'"); + expect(filterRefusalOf('{TODAY() - 3}')).toContain("{ dialect: 'cel', source: 'isoDate(daysAgo(3))' }"); + }); + + it('a token that resembles a query placeholder names it — a near-miss and the placeholder written with spaces', () => { + expect(filterRefusalOf('{current_user}')).toContain('If you meant the query placeholder `{current_user_id}`, write it so'); + expect(filterRefusalOf('{ today }')).toContain('If you meant the query placeholder `{today}`, write it so'); + // A record page's placeholder resolves on no server path: no such suggestion. + expect(filterRefusalOf('{record.id}')).not.toContain('query placeholder'); + }); + + it('text with holes is one concatenation; an envelope operand is not judged here', () => { + expect(filterRefusalOf('Acme {region} deal')).toContain(`{ dialect: 'cel', source: "'Acme ' + region + ' deal'" }`); + expect(valueSlotTemplateRefusals({ dialect: 'cel', source: 'record.id' }, { filterOperand: true })).toEqual([]); + }); + + it('the door call judges each operand at its own path, with the filter label', () => { + const refusals = flowNodeValueTemplateRefusals('delete_record', { + objectName: 'deal', + filter: { owner: '{record.owner}', due: { $lt: '{today}' }, id: { $in: ['r1', '{x}'] }, status: 'open' }, + }); + expect(refusals.map((r) => [r.path, r.label, r.source])).toEqual([ + ['filter.owner', 'delete_record filter operand', '{record.owner}'], + ['filter.id.$in[1]', 'delete_record filter operand', '{x}'], + ]); + }); +}); + describe('ADR-0087 D2 — no measured spelling is converted; the refusal is what meets it', () => { it('the whole conversion chain, retired entries included, leaves every spelling as authored', () => { const fields = { diff --git a/packages/spec/src/automation/flow-value-slot-template.ts b/packages/spec/src/automation/flow-value-slot-template.ts index 80fc6567fe4..eb213bb7bd3 100644 --- a/packages/spec/src/automation/flow-value-slot-template.ts +++ b/packages/spec/src/automation/flow-value-slot-template.ts @@ -14,8 +14,8 @@ * `defaults.*`, `fields[].defaultValue`, `recordId` — the list a `loop` * and a `map` iterate, their `collection` — a `notify` node's computed * values — `recipients` (each entry of a list), `templateData.*` and - * `sourceId` — and an `http` node's `body` — `FLOW_NODE_EXPRESSION_PATHS`), - * plus the two legacy + * `sourceId` — an `http` node's `body` — and each operand of a CRUD node's + * `filter` — `FLOW_NODE_EXPRESSION_PATHS`), plus the two legacy * `assignment` shapes the executor still reads (the * `assignments: [{ variable, value }]` array and the bare * `{ : }` config). Until this retirement every string @@ -181,6 +181,32 @@ * whole token holding a list, written as a list entry, on as ONE recipient * joined with commas. * + * ## A CRUD node's filter operands — the query placeholders stay + * + * Each operand of a `get_record`, `update_record` or `delete_record` node's + * `filter` is a value slot (the ledger's `filterOperands`, read through + * `flowFilterOperands`): a field's value, an operator's comparand, each element + * of a list comparand. Two `{…}` vocabularies met there, and the 17.x executor + * told them apart at run time: a flow token resolved against the run's + * variables, and a whole-string token no variable resolved that IS a query + * placeholder (`{current_user_id}`, `{today}`, `{30_days_ago}` — + * `isKnownFilterToken`) was handed to the query engine untouched. The + * placeholders stay — they are the query-filter surface, which the query + * engine resolves and ADR-0032 puts out of its scope — and the flow token is + * retired, so the split is now static and is drawn by ONE classifier, + * {@link classifyFlowFilterOperandString}: a whole-string placeholder the query + * engine resolves passes, and every other string carrying a `{…}` token is + * refused with its CEL spelling (judged with + * {@link ValueSlotTemplateOptions.filterOperand}). A flow variable that shares + * a placeholder's name no longer shadows it: the placeholder is the query + * engine's, and the variable is read with an envelope. + * + * Where a whole token resolved to nothing, the 17.x executor refused the node + * — an absent condition does not narrow a query, it widens it (#3810) — and so + * does an operand envelope that evaluates to `null`. So the remedy at a filter + * operand names no guard that answers `null`: where the value may be absent, + * the node is routed around. + * * ## The token grammar is the interpolator's * * Which `{…}` the interpolator substitutes, and how it dispatches a token, is @@ -190,6 +216,10 @@ */ import { isExpressionEnvelopeShaped, resolveFlowNodeValueSlots } from './flow-node-expression-paths'; +// [#19939] The query placeholders a flow filter operand hands on untouched — +// the vocabulary the query engine's own resolver reads (`resolveFilterTokens` +// in `@objectstack/core`), never re-spelled here. +import { FILTER_TOKEN_WRAPPED_RE, classifyFilterToken, isKnownFilterToken } from '../data/context-tokens.zod'; import { CEL_KEYWORDS, CEL_RUN_USER_ID, @@ -475,6 +505,104 @@ function collectionRemedy(value: string, tokens: readonly Token[]): string { ); } +/** + * [#19939] What a string at an operand of a CRUD node's `filter` is — the ONE + * classifier the node contracts, `AutomationEngine.registerFlow`, + * `objectstack validate` and the executors' contract parse all read, through + * {@link valueSlotTemplateRefusals} (see the module docblock, "A CRUD node's + * filter operands"): + * + * - `'placeholder'` — the whole string is ONE query placeholder the query + * engine resolves on the server, a context token or a date macro + * (`isKnownFilterToken`), in the wrapped spelling its resolver recognises + * (`FILTER_TOKEN_WRAPPED_RE`: `{today}`, `${today}`). Handed to the query + * untouched, as the 17.x executor handed a placeholder no flow variable + * resolved — and now whatever the run's variables hold: no flow token is + * read here, so none shadows it; + * - `'flow-token'` — any other string carrying a `{…}` token of the retired + * flow template dialect (`TEMPLATE_TOKEN`). Refused, with its CEL spelling; + * - `'literal'` — a string with no `{…}` token, handed to the query as + * written. + * + * A record page's placeholder (`{record_id}`) is not one the server resolves + * — the query engine refuses it on every server path — so here it is a token + * of the flow dialect like any other, as the 17.x executor read it. + */ +export type FlowFilterOperandStringKind = 'placeholder' | 'flow-token' | 'literal'; + +/** Classify one string at a filter operand — see {@link FlowFilterOperandStringKind}. */ +export function classifyFlowFilterOperandString(value: string): FlowFilterOperandStringKind { + const wrapped = FILTER_TOKEN_WRAPPED_RE.exec(value); + if (wrapped !== null && isKnownFilterToken(wrapped[1]!)) return 'placeholder'; + return tokensOf(value).length > 0 ? 'flow-token' : 'literal'; +} + +/** + * The query placeholder a refused whole-string token at a filter operand + * resembles, when it resembles one the server resolves — the placeholder + * written with spaces inside its braces, or a near-miss the placeholder + * vocabulary's own suggestion table names (`{current_user}` → + * `{current_user_id}`) — or `undefined`. + */ +function placeholderSuggestion(value: string): string | undefined { + const wrapped = FILTER_TOKEN_WRAPPED_RE.exec(value); + if (wrapped === null) return undefined; + const trimmed = wrapped[1]!.trim(); + if (trimmed !== wrapped[1] && isKnownFilterToken(trimmed)) return `{${trimmed}}`; + const verdict = classifyFilterToken(value); + if (verdict?.kind !== 'unknown' || verdict.suggestion === undefined) return undefined; + return isKnownFilterToken(verdict.suggestion) ? `{${verdict.suggestion}}` : undefined; +} + +/** + * What an operand envelope that evaluates to nothing does — the #3810 refusal, + * kept — and so why a filter operand's remedy names no guard answering `null`. + */ +const FILTER_OPERAND_ABSENT_SENTENCE = + 'An operand that evaluates to `null` refuses the node, as a token that resolved to nothing did — an absent ' + + 'condition does not narrow a query, it widens it — and CEL refuses an absent variable or key outright, so ' + + 'where the value may be absent, route around the node with a `decision` rather than guarding it.'; + +/** + * The remedy for a `{…}` token at a filter operand (see the module docblock, + * "A CRUD node's filter operands"): the operand's CEL envelope, evaluated + * before the query, and — where the value may be absent — what `null` does + * there, since a guard answering `null` refuses the node; then the query + * placeholder the token resembles, if any. + */ +function filterOperandRemedy(value: string, tokens: readonly Token[]): string { + const whole = wholeTokenOf(value, tokens); + const sentences: string[] = []; + if (whole?.kind === 'path') { + sentences.push( + `Write \`${whole.text}\` as ${envelopeOf(celPath(whole.inner))}: an operand envelope is evaluated before the ` + + 'query.', + FILTER_OPERAND_ABSENT_SENTENCE, + ); + } else if (whole?.kind === 'expression') { + sentences.push( + `Write \`${whole.text}\` as ${envelopeOf(celExpression(whole.inner))}: an operand envelope is evaluated before ` + + 'the query. Every division keeps a decimal operand: CEL divides two integers as integers, so ' + + '`round(x * 100) / 100` drops the decimals where `round(x * 100) / 100.0` keeps them.', + FILTER_OPERAND_ABSENT_SENTENCE, + ); + } else if (whole?.kind === 'user' && isRunUserIdToken(whole.inner)) { + sentences.push( + `Write \`${whole.text}\` as ${envelopeOf(CEL_RUN_USER_ID)}: \`current_user\` is the run's user. In a flow that ` + + 'can run without a user (a schedule, or a record change made by a system write) `current_user` is `null` ' + + 'and that read fails the run, so skip the node there with a `decision` on `current_user != null`: an ' + + 'operand that evaluates to `null` refuses the node.', + ); + } else { + sentences.push(remedyFor(value, tokens, envelopeOf)); + } + const placeholder = placeholderSuggestion(value); + if (placeholder !== undefined) { + sentences.push(`If you meant the query placeholder \`${placeholder}\`, write it so: the query engine resolves it.`); + } + return sentences.join(' '); +} + /** One refused string inside a value slot's value. */ export interface ValueSlotTemplateRefusal { /** Where the string sits inside the value — `[]` for the value itself, `['meta', 'note']`, `[0]`. */ @@ -506,6 +634,15 @@ export interface ValueSlotTemplateOptions { * are left to the collection's contract. Default `false`. */ readonly collection?: boolean; + /** + * [#19939] The position is an operand of a CRUD node's `filter` (the + * ledger's `filterOperands`): a string that is one whole query placeholder + * ({@link classifyFlowFilterOperandString}) is the query engine's and is not + * judged, at any depth, and every other `{…}` token is refused with the + * filter operand's remedy, which names no guard answering `null` — an + * operand that evaluates to `null` refuses the node. Default `false`. + */ + readonly filterOperand?: boolean; } /** @@ -532,16 +669,20 @@ export function valueSlotTemplateRefusals( return [{ path: [], message: `${VALUE_SLOT_TEMPLATE_REFUSAL} ${collectionRemedy(value, tokens)}`, source: value }]; } const legacy = options.envelopeIsLiteral === true; + const filter = options.filterOperand === true; if (!legacy && isExpressionEnvelopeShaped(value)) return []; const out: ValueSlotTemplateRefusal[] = []; const seen = new Set(); const visit = (node: unknown, path: (string | number)[]): void => { if (typeof node === 'string') { + // [#19939] At a filter operand a whole query placeholder is the query + // engine's, handed on untouched — the one classifier decides. + if (filter && classifyFlowFilterOperandString(node) === 'placeholder') return; const tokens = tokensOf(node); if (tokens.length === 0) return; const lead = literalPositionLead(node, tokens, path, legacy); const remedy = lead === undefined - ? remedyFor(node, tokens, envelopeOf) + ? (filter ? filterOperandRemedy(node, tokens) : remedyFor(node, tokens, envelopeOf)) : `${lead} ${remedyFor(node, tokens, path.length > 0 ? spanOf : envelopeOf)}`; out.push({ path, message: `${VALUE_SLOT_TEMPLATE_REFUSAL} ${remedy}`, source: node }); return; @@ -769,7 +910,10 @@ function joinPath(prefix: string, inner: readonly (string | number)[]): string { * (with a `body`) or a `map`'s `collection` only the value itself is judged, * with the collection's remedy: the list it must evaluate to. At a `notify` * node's `recipients` — the value, or each entry of a list — it ends with the - * recipient sentence: what `null` and a list add to the audience. + * recipient sentence: what `null` and a list add to the audience. At each + * operand of a CRUD node's `filter` a whole query placeholder passes, and a + * token is refused with the filter operand's remedy + * ({@link ValueSlotTemplateOptions.filterOperand}). */ export function flowNodeValueTemplateRefusals(nodeType: string, config: unknown): FlowNodeValueTemplateRefusal[] { const out: FlowNodeValueTemplateRefusal[] = []; @@ -792,6 +936,10 @@ export function flowNodeValueTemplateRefusals(nodeType: string, config: unknown) judge(found.path, found.entry.label, found.value, { collection: true }); continue; } + if (found.entry.filterOperands) { + judge(found.path, found.entry.label, found.value, { filterOperand: true }); + continue; + } const receiver = CALLEE_INPUT_SLOTS[entryKey]; const screenSlot = SCREEN_VALUE_SLOTS[entryKey]; // The map's own key (`input.`) — a callee entry's path ends in its `*`. diff --git a/packages/spec/src/automation/flow-write-node-stored-metadata-target.test.ts b/packages/spec/src/automation/flow-write-node-stored-metadata-target.test.ts index 491d67bbef4..6e89fd8bde1 100644 --- a/packages/spec/src/automation/flow-write-node-stored-metadata-target.test.ts +++ b/packages/spec/src/automation/flow-write-node-stored-metadata-target.test.ts @@ -45,9 +45,9 @@ type WriteNode = (typeof WRITE_NODES)[number]; /** A whole, valid config for `nodeType` aimed at `objectName` — the accept control's shape. */ function configFor(nodeType: WriteNode | 'get_record', objectName: unknown): Config { if (nodeType === 'create_record') return { objectName, fields: { name: 'pin' } }; - if (nodeType === 'update_record') return { objectName, filter: { id: '{record.id}' }, fields: { state: 'archived' } }; - if (nodeType === 'delete_record') return { objectName, filter: { id: '{record.id}' } }; - return { objectName, filter: { id: '{record.id}' }, outputVariable: 'row' }; + if (nodeType === 'update_record') return { objectName, filter: { id: { dialect: 'cel', source: 'record.id' } }, fields: { state: 'archived' } }; + if (nodeType === 'delete_record') return { objectName, filter: { id: { dialect: 'cel', source: 'record.id' } } }; + return { objectName, filter: { id: { dialect: 'cel', source: 'record.id' } }, outputVariable: 'row' }; } /** start → the probe node → end. */ diff --git a/packages/spec/src/automation/flow.test.ts b/packages/spec/src/automation/flow.test.ts index 95979ee1a97..bed027a672c 100644 --- a/packages/spec/src/automation/flow.test.ts +++ b/packages/spec/src/automation/flow.test.ts @@ -447,7 +447,7 @@ describe('FlowSchema', () => { // `opportunity.*` predicate below referenced nothing. config: { objectName: 'opportunity', - filter: { id: '{opportunityId}' }, + filter: { id: { dialect: 'cel', source: 'opportunityId' } }, outputVariable: 'opportunity', }, position: { x: 100, y: 150 }, @@ -474,7 +474,7 @@ describe('FlowSchema', () => { // outright — so this fixture could not have run as written either. config: { objectName: 'opportunity', - filter: { id: '{opportunityId}' }, + filter: { id: { dialect: 'cel', source: 'opportunityId' } }, fields: { status: 'approved', approved_by: 'system', @@ -660,10 +660,10 @@ describe('FlowSchema', () => { // string"), and `DAYS_AGO()` is a function no layer implements. // The date window is spelled in the dialect that OWNS a filter // value position: `{90_days_ago}` is a spec date macro - // (`DATE_MACRO_PARAM_RE`), and `interpolateFilter` hands a known - // filter token through VERBATIM for the query engine's - // `resolveFilterTokens` to expand (#3810 ownership transfer) — - // `date-macros.zod.ts` names "flow node filters" as a consumer. + // (`DATE_MACRO_PARAM_RE`), a query placeholder a flow filter + // operand hands to the query untouched for the query engine's + // `resolveFilterTokens` to expand (`classifyFlowFilterOperandString`) + // — `date-macros.zod.ts` names "flow node filters" as a consumer. // • `limit` was absent. The executor branches on it: `limit > 1` // runs `find` and outputs a `records` LIST; otherwise `findOne` // and a single `record`. A cleanup sweep wants the list, and the @@ -712,7 +712,7 @@ describe('FlowSchema', () => { // body, which is what makes `item` actually bound per iteration. config: { objectName: 'log_entry', - filter: { id: '{item.id}' }, + filter: { id: { dialect: 'cel', source: 'item.id' } }, }, }, ], @@ -1121,7 +1121,7 @@ describe('BPMN — Parallel Gateway & Join Gateway', () => { { id: 'finance_review', type: 'connector_action', label: 'Finance Review', connectorConfig: { connectorId: 'finance_desk', actionId: 'request_review' } }, { id: 'legal_review', type: 'connector_action', label: 'Legal Review', connectorConfig: { connectorId: 'legal_desk', actionId: 'request_review' } }, { id: 'join', type: 'join_gateway', label: 'Join — All Approved' }, - { id: 'final_approve', type: 'update_record', label: 'Final Approve', config: { objectName: 'contract', filter: { id: '{record.id}' }, fields: { status: 'approved' } } }, + { id: 'final_approve', type: 'update_record', label: 'Final Approve', config: { objectName: 'contract', filter: { id: { dialect: 'cel', source: 'record.id' } }, fields: { status: 'approved' } } }, { id: 'end', type: 'end', label: 'End' }, ], edges: [ @@ -1208,9 +1208,9 @@ describe('BPMN — Default Sequence Flow (isDefault)', () => { nodes: [ { id: 'start', type: 'start', label: 'Start' }, { id: 'check_priority', type: 'decision', label: 'Check Priority' }, - { id: 'high_path', type: 'update_record', label: 'High Priority Handler', config: { objectName: 'ticket', filter: { id: '{record.id}' } } }, - { id: 'medium_path', type: 'update_record', label: 'Medium Priority Handler', config: { objectName: 'ticket', filter: { id: '{record.id}' } } }, - { id: 'default_path', type: 'update_record', label: 'Default Handler', config: { objectName: 'ticket', filter: { id: '{record.id}' } } }, + { id: 'high_path', type: 'update_record', label: 'High Priority Handler', config: { objectName: 'ticket', filter: { id: { dialect: 'cel', source: 'record.id' } } } }, + { id: 'medium_path', type: 'update_record', label: 'Medium Priority Handler', config: { objectName: 'ticket', filter: { id: { dialect: 'cel', source: 'record.id' } } } }, + { id: 'default_path', type: 'update_record', label: 'Default Handler', config: { objectName: 'ticket', filter: { id: { dialect: 'cel', source: 'record.id' } } } }, { id: 'end', type: 'end', label: 'End' }, ], edges: [ @@ -1637,7 +1637,7 @@ describe('BPMN — Boundary Event', () => { errorCode: 'TIMEOUT', }, }, - { id: 'handle_error', type: 'update_record', label: 'Log Error', config: { objectName: 'sync_log', filter: { id: '{record.id}' } } }, + { id: 'handle_error', type: 'update_record', label: 'Log Error', config: { objectName: 'sync_log', filter: { id: { dialect: 'cel', source: 'record.id' } } } }, { id: 'end_success', type: 'end', label: 'End Success' }, { id: 'end_error', type: 'end', label: 'End Error' }, ], diff --git a/packages/spec/src/automation/flow.zod.ts b/packages/spec/src/automation/flow.zod.ts index 56d0a536899..226a5605c97 100644 --- a/packages/spec/src/automation/flow.zod.ts +++ b/packages/spec/src/automation/flow.zod.ts @@ -979,7 +979,7 @@ export const FlowEdgeSchema = lazySchema(() => strictObject( * { id: "start", type: "start", label: "Start", position: {x: 0, y: 0} }, * { id: "check_amount", type: "decision", label: "Check Amount", position: {x: 0, y: 100} }, * { id: "auto_approve", type: "update_record", label: "Auto Approve", position: {x: -100, y: 200}, - * config: { objectName: "order", filter: { id: "{record.id}" }, fields: { status: "approved" } } }, + * config: { objectName: "order", filter: { id: { dialect: "cel", source: "record.id" } }, fields: { status: "approved" } } }, * { id: "submit_for_approval", type: "connector_action", label: "Submit", position: {x: 100, y: 200}, * connectorConfig: { connectorId: "approvals_desk", actionId: "submit", input: { orderId: "{record.id}" } } } * ], diff --git a/packages/spec/src/automation/schemaless-node-config.zod.ts b/packages/spec/src/automation/schemaless-node-config.zod.ts index d11687de480..635efebc0e9 100644 --- a/packages/spec/src/automation/schemaless-node-config.zod.ts +++ b/packages/spec/src/automation/schemaless-node-config.zod.ts @@ -117,7 +117,9 @@ import { flowBoundVariableNameSchema } from './flow-bound-variable-name'; import { AssignmentConfigSchema, CreateRecordConfigSchema, + DeleteRecordConfigSchema, FlowValueSlotSchema, + GetRecordConfigSchema, MapConfigSchema, ScreenConfigSchema, UpdateRecordConfigSchema, @@ -652,7 +654,8 @@ export type SchemalessNodeType = keyof typeof SCHEMALESS_NODE_CONFIG_SCHEMAS; * Node types that DO publish a descriptor `configSchema` and still declare an * expression slot through a spec Zod, because the descriptor cannot carry the * marker (#14149; the CRUD pair since #19938; `map`, `screen`, `notify` and - * `http` since #19939). + * `http` since #19939, and the other two CRUD nodes, `get_record` and + * `delete_record`, since #19939's filter operands). * * Each of these descriptors declares its map as `additionalProperties: true` * — `assignment`'s `assignments`, `create_record` / `update_record`'s @@ -672,7 +675,11 @@ export type SchemalessNodeType = keyof typeof SCHEMALESS_NODE_CONFIG_SCHEMAS; * recipients picker and text inputs), so their markers ride * `NotifyConfigSchema`. `http`'s one value slot, `body`, is offered by its * descriptor as an untyped input with no marker, so its marker rides - * `HttpConfigSchema`. + * `HttpConfigSchema`. The three CRUD nodes that match rows — + * `get_record`, `update_record`, `delete_record` — offer `filter` as an open + * map (`additionalProperties: true`) with no marker, so the marker on its + * operands rides the contract's `filter` itself (one ledger entry each, whose + * `filterOperands` flag makes the walks emit each operand). * `map`'s other expression slot, `collection`, keeps arriving through its * descriptor (`xExpression: 'value'` since #19939 made it a value slot; it * was `'template'`), and so does `screen`'s @@ -681,7 +688,7 @@ export type SchemalessNodeType = keyof typeof SCHEMALESS_NODE_CONFIG_SCHEMAS; * from {@link SCHEMALESS_NODE_CONFIG_SCHEMAS} on purpose: that map means * "publishes no descriptor", its other readers (`metadata-protocol`'s * reference-site attribution) walk it for that reason, and none of these - * seven is a member of that class. + * nine is a member of that class. */ export const LEDGER_DECLARED_NODE_CONFIG_SCHEMAS = { assignment: AssignmentConfigSchema, @@ -691,6 +698,8 @@ export const LEDGER_DECLARED_NODE_CONFIG_SCHEMAS = { screen: ScreenConfigSchema, notify: NotifyConfigSchema, http: HttpConfigSchema, + get_record: GetRecordConfigSchema, + delete_record: DeleteRecordConfigSchema, } as const satisfies Record; /** Node types whose expression slots reach the ledger through {@link LEDGER_DECLARED_NODE_CONFIG_SCHEMAS}. */ diff --git a/packages/spec/src/data/context-tokens.zod.ts b/packages/spec/src/data/context-tokens.zod.ts index f3be2a0764b..806b358a92b 100644 --- a/packages/spec/src/data/context-tokens.zod.ts +++ b/packages/spec/src/data/context-tokens.zod.ts @@ -227,10 +227,13 @@ export const CONTEXT_TOKEN_WRAPPED_RE = DATE_MACRO_WRAPPED_RE; * braces (`'acme {x} deal'`) is untouched, as are `{a}{b}` and `{{x}}`, which * are not one wrapped token. * - * The flow template engine's filter position already used exactly this shape - * (`interpolateFilter` in `@objectstack/service-automation`, #3810) to decide - * "is this string one whole token?" before consulting the vocabulary — this - * aligns the platform diagnostic with the recognition rule that surface had. + * A flow CRUD node's filter operand reads exactly this shape too + * (`classifyFlowFilterOperandString` in `@objectstack/spec/automation`): a + * value that is one whole wrapped token the server resolves is a query + * placeholder, handed to the query untouched, and every other `{…}` token + * there is the retired flow template dialect, refused. (The 17.x flow executor + * used this shape to decide "is this string one whole token?" before + * consulting the vocabulary; this diagnostic aligned with it.) */ export const FILTER_TOKEN_WRAPPED_RE = /^\$?\{([^{}]+)\}$/; @@ -319,12 +322,13 @@ export const CONTEXT_TOKEN_SUGGESTIONS: Readonly name: 'cta_default', label: 'CTA', type: 'autolaunched', nodes: [ { id: 'start', type: 'start', label: 'Start', config: { objectName: 'piece', triggerType: 'record-after-create', condition: 'record.target_channels != null' } }, - { id: 'stamp', type: 'update_record', label: 'Stamp', config: { objectName: 'piece', filter: { id: '{record.id}' }, fields: { stamp: { dialect: 'cel', source: 'record.target_channels[0]' } } } }, + { id: 'stamp', type: 'update_record', label: 'Stamp', config: { objectName: 'piece', filter: { id: { dialect: 'cel', source: 'record.id' } }, fields: { stamp: { dialect: 'cel', source: 'record.target_channels[0]' } } } }, { id: 'end', type: 'end', label: 'End' }, ], edges: [ { id: 'e1', source: 'start', target: 'stamp' }, { id: 'e2', source: 'stamp', target: 'end' } ], diff --git a/packages/triggers/trigger-record-change/src/record-change-integration.test.ts b/packages/triggers/trigger-record-change/src/record-change-integration.test.ts index c52185c5f2c..8c791676e40 100644 --- a/packages/triggers/trigger-record-change/src/record-change-integration.test.ts +++ b/packages/triggers/trigger-record-change/src/record-change-integration.test.ts @@ -279,7 +279,7 @@ function stampFlow(name: string, object: string) { type: 'autolaunched', nodes: [ { id: 'start', type: 'start', label: 'Start', config: { objectName: object, triggerType: 'record-after-create' } }, - { id: 'stamp', type: 'update_record', label: 'Stamp', config: { objectName: object, filter: { id: '{record.id}' }, fields: { stamp: 'done' } } }, + { id: 'stamp', type: 'update_record', label: 'Stamp', config: { objectName: object, filter: { id: { dialect: 'cel', source: 'record.id' } }, fields: { stamp: 'done' } } }, { id: 'end', type: 'end', label: 'End' }, ], edges: [ @@ -303,7 +303,7 @@ function mirrorWriteFlow(name: string, object: string) { type: 'record_change', nodes: [ { id: 'start', type: 'start', label: 'Start', config: { objectName: object, triggerType: 'record-after-write' } }, - { id: 'mirror', type: 'update_record', label: 'Mirror', config: { objectName: object, filter: { id: '{record.id}' }, fields: { mirror: { dialect: 'cel', source: 'record.status' } } } }, + { id: 'mirror', type: 'update_record', label: 'Mirror', config: { objectName: object, filter: { id: { dialect: 'cel', source: 'record.id' } }, fields: { mirror: { dialect: 'cel', source: 'record.status' } } } }, { id: 'end', type: 'end', label: 'End' }, ], edges: [ @@ -338,7 +338,7 @@ function urgentAlertFlow(name: string, object: string) { condition: "priority == 'urgent' && (previous == null || previous.priority != 'urgent')", }, }, - { id: 'alert', type: 'update_record', label: 'Alert', config: { objectName: object, filter: { id: '{record.id}' }, fields: { alerted: 'yes' } } }, + { id: 'alert', type: 'update_record', label: 'Alert', config: { objectName: object, filter: { id: { dialect: 'cel', source: 'record.id' } }, fields: { alerted: 'yes' } } }, { id: 'end', type: 'end', label: 'End' }, ], edges: [ diff --git a/packages/triggers/trigger-record-change/src/reentrant-start-condition.test.ts b/packages/triggers/trigger-record-change/src/reentrant-start-condition.test.ts index 48a4baa8420..295ea9066a9 100644 --- a/packages/triggers/trigger-record-change/src/reentrant-start-condition.test.ts +++ b/packages/triggers/trigger-record-change/src/reentrant-start-condition.test.ts @@ -225,7 +225,7 @@ const escalationFlow = (name: string, object: string) => ({ // whole point: the author's guard is what should stop the re-fire. config: { objectName: object, - filter: { id: '{record.id}' }, + filter: { id: { dialect: 'cel', source: 'record.id' } }, fields: { status: 'escalated', escalated_date: '2026-08-14T13:14:38.628Z' }, }, }, diff --git a/packages/verify/src/handle.fixture.ts b/packages/verify/src/handle.fixture.ts index 192557c72e1..1742edabd60 100644 --- a/packages/verify/src/handle.fixture.ts +++ b/packages/verify/src/handle.fixture.ts @@ -187,7 +187,7 @@ export const resolveNoteFlow: Flow = { label: 'Apply resolution', config: { objectName: 'hnd_note', - filter: { id: '{noteId}' }, + filter: { id: { dialect: 'cel', source: 'noteId' } }, // A CEL value envelope — the `{…}` template dialect is retired from // value slots; `resolution` is the screen's required field. fields: { status: 'resolved', resolution: { dialect: 'cel', source: 'resolution' } }, diff --git a/packages/verify/src/handle.system-insert-delete.test.ts b/packages/verify/src/handle.system-insert-delete.test.ts index 5ea122e2b7d..0f37452af0a 100644 --- a/packages/verify/src/handle.system-insert-delete.test.ts +++ b/packages/verify/src/handle.system-insert-delete.test.ts @@ -153,7 +153,7 @@ function deleteReader(name: string, triggerType: 'record-before-delete' | 'recor id: 'read', type: 'get_record', label: 'Read the case back', - config: { objectName: CASE, filter: { id: '{record.id}' }, outputVariable: 'held' }, + config: { objectName: CASE, filter: { id: { dialect: 'cel', source: 'record.id' } }, outputVariable: 'held' }, }, ledgerWrite(name, { previous_status: { dialect: 'cel', source: 'previous.status' }, diff --git a/skills/objectstack-automation/references/_index.md b/skills/objectstack-automation/references/_index.md index c0d5a76f2fd..51c2c60c649 100644 --- a/skills/objectstack-automation/references/_index.md +++ b/skills/objectstack-automation/references/_index.md @@ -23,6 +23,8 @@ from `node_modules` — there is no local copy in the skill bundle. - `node_modules/@objectstack/spec/src/automation/control-flow.zod.ts` — Structured control-flow constructs (ADR-0031) — the **native + AI-authored** - `node_modules/@objectstack/spec/src/automation/schemaless-node-config.zod.ts` — Config contracts for the **descriptor-schemaless** builtins whose designer +- `node_modules/@objectstack/spec/src/data/context-tokens.zod.ts` — Context Tokens — the declarative placeholders that resolve against the +- `node_modules/@objectstack/spec/src/data/date-macros.zod.ts` — Date Macro Tokens — the declarative placeholders the UI substitutes - `node_modules/@objectstack/spec/src/kernel/metadata-protection.zod.ts` — Metadata Protection Model — Phase 1 (ADR-0010) - `node_modules/@objectstack/spec/src/shared/expression.zod.ts` — Expression Protocol - `node_modules/@objectstack/spec/src/shared/identifiers.zod.ts` — Exports: SystemIdentifierSchema, SnakeCaseIdentifierSchema, MetadataItemNameSchema