diff --git a/.changeset/19939-flow-single-brace-resolver-deleted.md b/.changeset/19939-flow-single-brace-resolver-deleted.md new file mode 100644 index 00000000000..647fcaef82a --- /dev/null +++ b/.changeset/19939-flow-single-brace-resolver-deleted.md @@ -0,0 +1,45 @@ +--- +'@objectstack/service-automation': major +'@objectstack/lint': major +'@objectstack/spec': patch +'@objectstack/verify': patch +--- + +No flow position reads the single-brace `{…}` template dialect any more, and its resolver is deleted (ADR-0032 Sequencing 6: "delete the single-brace resolver"). `AutomationEngine.evaluateCondition` evaluates every condition as CEL: a condition carrying a `{var}` hole is refused as the CEL brace trap, naming its source, and a `template` or `flow` envelope is refused before evaluation, where the deleted path substituted the variables and compared the text. A literal in a value slot is handed on as written: a `Date` in a code-defined flow reaches the write as the `Date`, where the deleted interpolator wrote `{}`. + +Clause-②: no (narrowing) + + + +**BREAKING**: a narrowing of a published runtime method and of published lint exports, 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 3 keeps one template delimiter, `{{ }}`, and its Sequencing 6 deletes the single-brace resolver. The earlier changes on this line retired the dialect position by position — every value slot, the text slots, each CRUD filter operand — so the resolver's last readers were two calls that saw literals only and a condition path that only a direct call reached. + +**What changes.** + +- **Conditions.** `AutomationEngine.evaluateCondition(condition, variables, context?)` reads every condition as CEL — bare text, an envelope with `dialect: 'cel'`, or one naming no dialect. The template path it kept for a string carrying a `{var}` hole is deleted: such a string is refused as a CEL fault that names its source and says not to wrap field references in `{…}`, and an envelope with `dialect: 'template'` or `dialect: 'flow'` is refused before evaluation. `registerFlow` already refused both at every condition slot (a start node's `condition`, an edge `condition`, a `decision` branch's `expression`, a screen field's `visibleWhen`), so no registered flow changes. What changes is a direct caller: a plugin calling the method, or `@objectstack/verify`'s `stack.automation.evaluateCondition`, which hands the condition over as given. +- **Value-slot literals.** A literal in a value slot — a `fields`, `input` or `templateData` value, an `assignment` value in any of its three shapes, a `notify` recipient, an `http` `body`, a screen's `defaults`, `defaultValue` or `recordId` — is handed on as a copy of what was written (arrays and plain objects copied, every other value as it is). The deleted interpolator walked every object's own entries, so a `Date` written in a code-defined flow reached the write as `{}`; it reaches it as the `Date`. +- **`@objectstack/lint`.** `validateFlowTemplatePaths` reads the `{{ }}` holes of a node's strings and no single-brace token; a CRUD node's `filter` is not scanned, since each operand is a value slot that renders no template. So `flow-template-unknown-field` and `flow-template-lookup-traversal` report only `warning` now: their `error` at a filter operand, and every finding on a single-brace token, are gone, and the value-slot and text-slot judges refuse those tokens at the same doors as `expression-invalid`. Messages and hints spell a hole `{{ record.x }}`. `flow-double-brace-interpolation` and `flow-bare-dollar-reference`, at a string outside every value slot, text slot and declared literal (a plugin node's own config, a `connector_action` input), say the value is read as written, where they prescribed a `{var}` token nothing reads. The lint's mirror of the retired grammar is removed with its exports. +- **`@objectstack/spec`.** The `FlowSchema` example's `connector_action` input holds a literal, since nothing reads a token there. The upgrade guide's step-18 text-slot entry says no other flow string reads the single-brace dialect, where it said every other one kept it, and its value-slot entry names every value slot the retirement reached on this line, where it named only the `assignment` values and the CRUD `fields` map. + +## FROM → TO + +| you wrote | write instead | what changes | +|:--|:--|:--| +| `engine.evaluateCondition('{amount} > 100', vars)` | `engine.evaluateCondition('amount > 100', vars)` | the template substituted text and compared it; CEL compares the values and fails the run on an absent variable | +| `"{status} == 'open'"`, `'{status} == open'` | `"status == 'open'"` | a string literal is quoted | +| `{ dialect: 'template', source: "{flag} == true" }`, `dialect: 'flow'` | `{ dialect: 'cel', source: 'flag == true' }` | — | +| `classifyFlowTemplateToken(inner)` (`@objectstack/lint`) | `valueSlotTemplateRefusals(value)` / `textSlotTemplateRefusal(text)` (`@objectstack/spec/automation`), which refuse a token with the spelling that replaces it | the grammar it classified is read by nothing | +| `FLOW_TEMPLATE_DATE_FUNCTIONS`, `FLOW_TEMPLATE_VALUE_FUNCTIONS` | the CEL stdlib: `isoDate(today())`, `isoDatetime(now())`, `round`, `floor`, `ceil`, `abs`, `min`, `max` | — | +| `FlowTemplateTokenVerdict` | nothing: the judges' reading of the retired grammar is internal to `@objectstack/spec` | — | + +**The one-line fix: write a condition as CEL, without `{…}` around a reference.** + +**Who is affected, measured.** In this repository at the base: no braced condition and no `template` or `flow` condition envelope in `examples/`, the docs or any package source; the deleted path's own pins in `@objectstack/service-automation` (`engine.test.ts`, `nested-region-parity.test.ts`) and `@objectstack/verify`'s door test are re-pinned to the refusal here. No authored `Date` literal in a flow was found; a compiled artifact serialises one to text, so only a flow handed to `registerFlow` in code carried one. Neither repository imports the removed lint exports. hotcrm at `0f84c9c` carries no braced condition and no `connector_action` input; its flows still author the single-brace dialect at value and text positions earlier changes on this line refuse, which its own upgrade migrates. + +### The kit + +- **The engine.** `AutomationEngine.evaluateCondition` (`@objectstack/service-automation`): the template path, its hole discriminator and its text comparison are deleted. The value-slot resolvers' literal arm and the `assignment` executor copy a literal (`copyLiteral`, `builtin/value-slot-map.ts`). The single-brace interpolator (`interpolate`, `interpolateString`, `resolveToken`, its function table and `FlowExpressionFunctionError`), which the package did not export, is deleted with its tests. +- **The lint.** `validateFlowTemplatePaths` and the two wrong-syntax hints (`lint-flow-patterns.ts`); `classifyFlowTemplateToken`, `FLOW_TEMPLATE_DATE_FUNCTIONS`, `FLOW_TEMPLATE_VALUE_FUNCTIONS` and `FlowTemplateTokenVerdict` are removed. +- **The verify handle.** `stack.automation.evaluateCondition`'s documentation states the engine's reading: CEL, a `{var}` hole refused. +- **ADR-0087.** No new entry: the step-18 D3 entries `flow-value-slot-template-dialect-refused` and `flow-text-slot-single-brace-refused` record the retirement. diff --git a/.changeset/pre/19939-flow-callee-input-value-slots.md b/.changeset/pre/19939-flow-callee-input-value-slots.md index 87351821745..4f5e2f9b9d6 100644 --- a/.changeset/pre/19939-flow-callee-input-value-slots.md +++ b/.changeset/pre/19939-flow-callee-input-value-slots.md @@ -29,7 +29,7 @@ Clause-②: no (narrowing) **Who is affected, measured.** In this repository: 5 values in 3 nodes of the showcase example (`showcase_task_completed`'s `script`, and the `subflow` nodes of `showcase_task_done_notify_owner` and `showcase_project_closure`), 3 values in the todo example's `task_completion` `script`, 2 docs pages, the service-automation README, 2 dogfood fixtures and the test fixtures; all are migrated in this change. hotcrm's sites at these positions were not measured. -**Still accepted, unchanged.** A CEL value envelope and every literal. The single-brace dialect keeps resolving where it still lives, a CRUD `filter` value. +**Still accepted, unchanged.** A CEL value envelope and every literal. ### The kit diff --git a/.changeset/pre/19939-flow-collection-value-slots.md b/.changeset/pre/19939-flow-collection-value-slots.md index fc850756ab7..b1360ef5146 100644 --- a/.changeset/pre/19939-flow-collection-value-slots.md +++ b/.changeset/pre/19939-flow-collection-value-slots.md @@ -35,7 +35,7 @@ Clause-②: yes (narrowing: a string is refused at loop.collection and map.colle **Newly accepted.** A CEL value envelope at `loop.collection` and `map.collection`: the published JSON Schema for both gains the envelope arm (a list, or the envelope object), and so do the inferred `LoopConfig['collection']` and `MapConfig['collection']`, which lose `string`. `FlowCollectionSlotSchema` (`@objectstack/spec/automation`) is the contract both take. -**Still accepted, unchanged.** An inline array at both, and on a body-less `loop` any `collection` its legacy form reads. The single-brace dialect keeps resolving where it still lives, a CRUD `filter` value. +**Still accepted, unchanged.** An inline array at both, and on a body-less `loop` any `collection` its legacy form reads. ### The kit diff --git a/.changeset/pre/19939-flow-crud-filter-operands.md b/.changeset/pre/19939-flow-crud-filter-operands.md index c6f671215f6..97b6f57a253 100644 --- a/.changeset/pre/19939-flow-crud-filter-operands.md +++ b/.changeset/pre/19939-flow-crud-filter-operands.md @@ -24,7 +24,7 @@ Clause-②: yes (narrowing: a flow {…} token at a get_record, update_record or **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. +**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. A consumer moves to the value-slot judge's findings: rule id `flow-filter-token-unknown` → `expression-invalid` (`EXPRESSION_INVALID`); `validateFlowFilterTokens(stack)` → `validateStackExpressions(stack)`, which reports each refused operand located at its node and `config.filter.`; `FlowFilterTokenFinding` → `ExprIssue`; `FLOW_FILTER_TOKEN_UNKNOWN` → `EXPRESSION_INVALID`. ## FROM → TO @@ -40,7 +40,7 @@ Clause-②: yes (narrowing: a flow {…} token at a get_record, update_record or **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. +**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. 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. diff --git a/.changeset/pre/19939-flow-http-slots.md b/.changeset/pre/19939-flow-http-slots.md index aa3c1a81503..a2481084956 100644 --- a/.changeset/pre/19939-flow-http-slots.md +++ b/.changeset/pre/19939-flow-http-slots.md @@ -41,7 +41,7 @@ Clause-②: yes (narrowing: a {…} token is refused at http url, each headers v **Newly refused in the body.** An object naming a `dialect` was data before and was sent as written; it is an expression now, evaluated per run, and a malformed one is refused. An envelope nested inside a literal body is still data, sent as written. -**Still accepted, unchanged.** Every literal at these keys, and the published shapes of `url` (a string), `headers` (a map of strings), `method`, `durable`, `timeoutMs` and `signingSecret`. The single-brace dialect keeps resolving where it still lives, a CRUD `filter` value. +**Still accepted, unchanged.** Every literal at these keys, and the published shapes of `url` (a string), `headers` (a map of strings), `method`, `durable`, `timeoutMs` and `signingSecret`. ### The kit diff --git a/.changeset/pre/19939-flow-notify-value-slots.md b/.changeset/pre/19939-flow-notify-value-slots.md index 35bc7dc4661..0e034db6910 100644 --- a/.changeset/pre/19939-flow-notify-value-slots.md +++ b/.changeset/pre/19939-flow-notify-value-slots.md @@ -41,7 +41,7 @@ Clause-②: yes (narrowing: a {…} token is refused at notify recipients (the v **Newly refused at the literals.** A `{…}` token in `sourceObject`, in `actorId` or in any string of a `payload` value, and a CEL value envelope written as a `payload` value, which was handed on as the object it spells. -**Still accepted, unchanged.** Every literal at these positions. The single-brace dialect keeps resolving where it still lives, a CRUD `filter` value. +**Still accepted, unchanged.** Every literal at these positions. ### The kit diff --git a/.changeset/pre/19939-flow-screen-value-slots.md b/.changeset/pre/19939-flow-screen-value-slots.md index 8060e80062d..edff5673343 100644 --- a/.changeset/pre/19939-flow-screen-value-slots.md +++ b/.changeset/pre/19939-flow-screen-value-slots.md @@ -38,7 +38,7 @@ The refusal at each position says what it serves. **Newly accepted.** A CEL value envelope at `recordId`, which `z.string()` refused before this change: the published JSON Schema for `recordId` gains the envelope arm (a string, or the envelope object), and so does the inferred `ScreenConfig['recordId']`. -**Still accepted, unchanged.** A CEL value envelope at `defaults` and at a field's `defaultValue`, and every literal at all three, a string `recordId` included. The single-brace dialect keeps resolving where it still lives, a CRUD `filter` value. +**Still accepted, unchanged.** A CEL value envelope at `defaults` and at a field's `defaultValue`, and every literal at all three, a string `recordId` included. ### The kit diff --git a/.changeset/pre/19939-flow-value-slot-date-macros-refused.md b/.changeset/pre/19939-flow-value-slot-date-macros-refused.md index 342c282bc4c..f40ecc1f646 100644 --- a/.changeset/pre/19939-flow-value-slot-date-macros-refused.md +++ b/.changeset/pre/19939-flow-value-slot-date-macros-refused.md @@ -44,7 +44,7 @@ Clause-②: no (narrowing) **Who is affected, measured.** This repository's examples carry no date macro in a value slot; its own test fixtures and docs that did are migrated in this change. The census taken for this retirement's first change measured 15 date-macro value-slot sites in hotcrm at `c529de2` (8 `{NOW()}`, 3 `{TODAY()}`, 3 `{TODAY() + N}`, 1 `{TODAY() + var}`); they migrate there. Deployed metadata and other repositories were not measured here. -**Still accepted, unchanged.** A CEL value envelope and every literal. The single-brace dialect, date macros included, keeps resolving where it still lives, a CRUD `filter` value. +**Still accepted, unchanged.** A CEL value envelope and every literal. ### The kit diff --git a/.changeset/pre/19939-flow-value-slot-run-user-refused.md b/.changeset/pre/19939-flow-value-slot-run-user-refused.md index 8bbf01c19c8..3eb82bdba1e 100644 --- a/.changeset/pre/19939-flow-value-slot-run-user-refused.md +++ b/.changeset/pre/19939-flow-value-slot-run-user-refused.md @@ -19,7 +19,7 @@ Clause-②: no (narrowing) **The expression remedy.** A refused `{…}` arithmetic token's CEL spelling now reads every variable path in it the way a lone path token's does — `{int * 2}` is `vars["int"] * 2`, `{items.0 * 2}` is `items[0] * 2` — instead of rewriting only its divisors, which printed envelopes that did not evaluate. -**Still accepted, unchanged.** A CEL value envelope and every literal. `{$User.*}` keeps resolving where the single-brace dialect still lives, a CRUD `filter` value. +**Still accepted, unchanged.** A CEL value envelope and every literal. ## FROM → TO @@ -27,7 +27,7 @@ Clause-②: no (narrowing) |:--|:--|:--| | `'{$User.Id}'` | `{ dialect: 'cel', source: 'current_user.id' }` | nothing when the run has a user | | `'{$User.Id}'`, in a flow that can run without a user | `{ dialect: 'cel', source: 'current_user != null ? current_user.id : null' }` — or skip the node with `current_user != null` as a start condition or a `decision` | the guarded form writes `null` where the template wrote nothing, so on `update_record` it clears a stored value the template left alone | -| `'{$User.Email}'`, `'{$User.Name}'`, any other `$User` path | an `assignment` of `uid: { dialect: 'cel', source: 'current_user.id' }`, a `get_record` on `sys_user` with `filter: { id: '{uid}' }` and `outputVariable: 'me'`, then `{ dialect: 'cel', source: 'me.email' }` | these never resolved in any shipped run: they wrote nothing | +| `'{$User.Email}'`, `'{$User.Name}'`, any other `$User` path | a `get_record` on `sys_user` with `filter: { id: { dialect: 'cel', source: 'current_user.id' } }` and `outputVariable: 'me'`, then `{ dialect: 'cel', source: 'me.email' }` | these never resolved in any shipped run: they wrote nothing | | `'Owner: {$User.Id}'` | `{ dialect: 'cel', source: "'Owner: ' + current_user.id" }` | in a user-less run, write the hole as `(current_user != null ? current_user.id : '')` | | a text slot's `assignments: { by: '{$User.Id}' }`, then `'By {{ by }}'` | `assignments: { by: { dialect: 'cel', source: 'current_user.id' } }`, then `'By {{ by }}'` | nothing when the run has a user; where the flow can run without one, compute `current_user != null ? current_user.id : null`, which renders nothing where the template did | @@ -41,5 +41,5 @@ Clause-②: no (narrowing) - **The refusal.** `valueSlotTemplateRefusals` / `flowNodeValueTemplateRefusals` (`@objectstack/spec/automation`); the text-slot judge's run-user remedy (`textSlotTemplateRefusal`) now computes the id with the CEL envelope. - **The ledger.** The step-18 D3 entries `flow-value-slot-template-dialect-refused` (amended to refuse the run-user paths), `flow-text-slot-single-brace-refused` and `flow-text-slot-unbound-dollar-root-refused` (their run-user remedy). No key is removed, so there is no tombstone, and there is no D2 conversion. - **`@objectstack/cli`.** `os explain flow`'s example writes `current_user.id` and gates on `current_user != null`. -- **`@objectstack/lint`.** `flow-bare-dollar-reference`'s hint for a bare `$name.path` in a value slot names the CEL envelope the value-slot refusal writes for it — `current_user.id` and its guard for `$User.Id`, the variable the reference names for any other — where it named `{source.id}` and `{$User.Id}`, both refused there. A text slot's hint keeps the `{{ }}` hole, the hint at a node's literal names the value itself, and every other position keeps the single brace. +- **`@objectstack/lint`.** `flow-bare-dollar-reference`'s hint for a bare `$name.path` in a value slot names the CEL envelope the value-slot refusal writes for it — `current_user.id` and its guard for `$User.Id`, the variable the reference names for any other — where it named `{source.id}` and `{$User.Id}`, both refused there. A text slot's hint keeps the `{{ }}` hole, and the hint at a node's literal, or at any other position, names the value itself. - **`@objectstack/service-automation`'s README.** Its *Expressions* section states the value-slot envelope and `current_user`. diff --git a/.changeset/pre/19939-flow-value-slot-template-dialect-refused.md b/.changeset/pre/19939-flow-value-slot-template-dialect-refused.md index b6d41f37c28..3a9b4b12ff9 100644 --- a/.changeset/pre/19939-flow-value-slot-template-dialect-refused.md +++ b/.changeset/pre/19939-flow-value-slot-template-dialect-refused.md @@ -18,7 +18,7 @@ Clause-②: yes (narrowing) **Not converted.** No D2 conversion rewrites any spelling: every token spelling authored in flows answers differently under CEL for some input (measured through both engines over the same variables, 13 of 25 probes the same, 12 different), so a rewrite would change what some flow writes. Where a value may be absent, whether the field should take nothing, `null` or a default was decided silently by the template; it is now the author's decision. -**What is still accepted, unchanged.** A CEL value envelope and every literal (a token-free string, numbers, booleans, `null`, arrays, objects). A `filter` value keeps the template dialect; a text slot (`notify` `title` / `message`, a screen `description`, …) reads `{{ }}` holes and refuses a single-brace token. +**What is still accepted, unchanged.** A CEL value envelope and every literal (a token-free string, numbers, booleans, `null`, arrays, objects). A text slot (`notify` `title` / `message`, a screen `description`, …) reads `{{ }}` holes and refuses a single-brace token. ## FROM → TO diff --git a/.changeset/pre/22110-flow-text-slot-double-brace.md b/.changeset/pre/22110-flow-text-slot-double-brace.md index 57494051418..9c8e07802a1 100644 --- a/.changeset/pre/22110-flow-text-slot-double-brace.md +++ b/.changeset/pre/22110-flow-text-slot-double-brace.md @@ -21,7 +21,7 @@ Clause-②: yes (narrowing) - **The refusal.** One judge in `@objectstack/spec/automation` (`textSlotTemplateRefusal`, `flowNodeTextSlotSources`, `FLOW_NODE_TEXT_SLOTS`, `TEXT_SLOT_TEMPLATE_REFUSAL`): `NotifyConfigSchema`, `ScreenConfigSchema` and `EndConfigSchema` refuse a single-brace token at the slot's key; `registerFlow` and `objectstack validate` (`expression-invalid`, `error`) refuse it with the same words and also compile every slot's holes, so `{{ a + b }}` or an unknown formatter is refused before a run. A stored flow carrying one is skipped at boot with a warn naming it. Past the doors, a hole that does not compile fails the node with a guard refusal, which a `fault` edge does not route. - **`@objectstack/formula`.** A template hole's path may contain `$` (`{{ $error.message }}`): a widening of the hole grammar, so a host scope's `$`-named variable has a template spelling. An unbound one renders nothing, like any unknown path. - **`flow-double-brace-interpolation`** no longer flags `{{ }}` on the text slots, and its hint names them as the only `{{ }}` positions; a bare `$ref.x` outside the holes of a text slot is flagged with the hole spelling. -- **Unchanged:** a CRUD `filter` value keeps the single-brace dialect, and the value slots are CEL's. +- **Unchanged:** the value slots are CEL's. **Not converted.** No ADR-0087 D2 conversion rewrites `{x}` to `{{ x }}`. The two renderers were run over the same variables: a path renders the same text for a string, a number, a boolean, `null`, an absent key or variable, an ISO date string, an object or an array, but a `Date` value rendered JSON-quoted under the 17.x interpolator (`"2026-10-08T09:30:00.000Z"`, quotes included) and as its ISO text now, and a screen or `end` text that was one token holding an object, an array or a `Date` rendered `String(value)` (`[object Object]`). So the rewrite is the author's to check; the D3 record is the semantic entry `flow-text-slot-single-brace-refused`. diff --git a/.changeset/pre/22161-lint-slice-10-one-line.md b/.changeset/pre/22161-lint-slice-10-one-line.md index ba5dd568b66..c3f0ff02e53 100644 --- a/.changeset/pre/22161-lint-slice-10-one-line.md +++ b/.changeset/pre/22161-lint-slice-10-one-line.md @@ -2,12 +2,12 @@ "@objectstack/lint": patch --- -fix(lint): the flow-credential, AI skill, flow-filter-token, managed-object API method, organization-axis business-unit grant, retired permission residue, seed and semantic-role provenance findings print one verdict line, `os explain ` carries their reasoning, and `field-no-consumers` stops listing every object that shares a field name +fix(lint): the flow-credential, AI skill, managed-object API method, organization-axis business-unit grant, retired permission residue, seed and semantic-role provenance findings print one verdict line, `os explain ` carries their reasoning, and `field-no-consumers` stops listing every object that shares a field name Clause-②: no -- **Shorter verdicts.** Each finding of these 10 rule ids now prints a `message` of one verdict sentence. Every finding the rules' own test suites fire is at most 194 characters, and the runtime publish gate's suite at most 181; before, the longest of each ran from 222 to 441 characters. The ids: - - flows: `flow-credential-literal` (a literal credential in an `http` node's headers or url query, or in a `connectorConfig.input`) and `flow-filter-token-unknown` (a `{NAME()}` filter token neither the flow template dialect nor the filter placeholders define); +- **Shorter verdicts.** Each finding of these 9 rule ids now prints a `message` of one verdict sentence. Every finding the rules' own test suites fire is at most 194 characters, and the runtime publish gate's suite at most 181; before, the longest of each ran from 222 to 441 characters. The ids: + - flows: `flow-credential-literal` (a literal credential in an `http` node's headers or url query, or in a `connectorConfig.input`); - AI agents and skills: `ai-skill-surface-mismatch` and `ai-skill-tool-unresolved`; - managed objects: `object/managed-api-method-unaffordable`; - sharing rules: `org-axis-cross-org-bu-grant` (its sibling `org-axis-permission-inheritance` already printed one sentence of at most 174 characters and is unchanged); @@ -15,8 +15,10 @@ Clause-②: no - seed datasets: `seed-insert-mode-duplicates-on-replay` and `seed-value-outside-state-machine`; - semantic roles: `semantic-role-field-unprovisioned`. + A tenth id this change shortened, `flow-filter-token-unknown`, is retired in this same release with its rule: a flow filter operand is a value slot, and a token there is refused as `expression-invalid` by `validateStackExpressions` (its `ExprIssue`), with the CEL envelope that replaces it. + A verdict no longer repeats what the finding's `where` already names: the skill (`ai-skill-tool-unresolved` now opens on the tool), the managed object, the sharing rule's object and the semantic-role object. `flow-credential-literal` keeps a short route (`move it to a connector's \`auth.credentialRef\``, or `drop it` for a connector input) because `os lint` prints the fix only under `--fix`; the route by auth variant stays the fix line's. `seed-value-outside-state-machine` names the first three declared states, then `(and N more)`. `object/managed-api-method-unaffordable` no longer appends the spec's `describeManagedApiMethodConflicts` sentence; it names the refused verbs, the `managedBy` bucket and the affordances they need. The `fix` (the CLI's `fix:` line, the runtime issue's `hint`), every rule id, severity and `path`, and what each rule accepts or refuses are unchanged. A tool that matched the old message text should match on `rule` and `path` instead. - **`field-no-consumers`** names the first other object that declares a field of the same name and counts the rest (`a consumer of the same name on "showcase_account" (and 9 more) does not count`), where it listed every one. On `os validate` of the showcase app its longest finding drops from 341 characters to 199, and that 341-character one, which named ten objects, now prints 143. Its `inert` and `carrier-only` verdicts and its explanation are otherwise unchanged. -- **`os explain ` takes these 10 ids**, for example `os explain flow-credential-literal`. It prints the reasoning the verdicts no longer carry: which flow positions are served to every flow reader and where a credential belongs, by auth variant; how an agent's and a skill's surfaces must match, and why an unresolved skill name is skipped; how a skill's tool name resolves and when an action materialises as a tool; the two vocabularies a flow filter token resolves in, and why a miss stops the node; which affordance each write verb needs and what the registry does with a refused one; why business-unit trees stay inside one organization and which recipients may share a platform-global object; why the retired `allowRestore` / `allowPurge` default is accepted in silence and which channels miss it; why seeds must be safe to replay and which states a seeded value may name; and why an injected column on an external object is blank. The `rule:` line under each of these findings now ends with `` — `os explain ` for … ``. The no-argument listing and its `--json` `rules` array list the 10 ids, and so does the unknown-id error's `Rules with an explanation:` line. `RULE_EXPLANATIONS` in `@objectstack/lint` gains the 10 entries. -- **Where the new text prints.** On the CLI, all 11 ids: `os validate`, `os build` (and `os compile`, which `os dev` runs when it compiles at startup, and on each watch recompile under `--verbose`), `os lint` (as `where: message`) and the scaffold check `os init` runs print the new `message` on the text face, and `os validate --json` and `os build --json` carry it in their `errors` and author-time `issues`. `os verify` prints the error-severity ids' new text in its refusal list (`ai-skill-surface-mismatch`, `flow-filter-token-unknown`, `object/managed-api-method-unaffordable`, `org-axis-cross-org-bu-grant`) and counts the warnings without printing them. At the runtime publish gate (Studio, REST `/meta`, MCP): a `flow` write's `flow-filter-token-unknown` and an `object` write's `object/managed-api-method-unaffordable` change the 422 issue's `message` and the refusal log line under `OS_ALLOW_UNLINTED_METADATA_WRITES`; a `flow` write's `flow-credential-literal` and a `permission` write's `permission-retired-lifecycle-residue` change the 2xx `advisories` entry and the `[Protocol] authoring advisory` log line. Each issue's `hint` is unchanged. +- **`os explain ` takes these 9 ids**, for example `os explain flow-credential-literal`. It prints the reasoning the verdicts no longer carry: which flow positions are served to every flow reader and where a credential belongs, by auth variant; how an agent's and a skill's surfaces must match, and why an unresolved skill name is skipped; how a skill's tool name resolves and when an action materialises as a tool; which affordance each write verb needs and what the registry does with a refused one; why business-unit trees stay inside one organization and which recipients may share a platform-global object; why the retired `allowRestore` / `allowPurge` default is accepted in silence and which channels miss it; why seeds must be safe to replay and which states a seeded value may name; and why an injected column on an external object is blank. The `rule:` line under each of these findings now ends with `` — `os explain ` for … ``. The no-argument listing and its `--json` `rules` array list the 9 ids, and so does the unknown-id error's `Rules with an explanation:` line. `RULE_EXPLANATIONS` in `@objectstack/lint` gains the 9 entries. +- **Where the new text prints.** On the CLI, all 10 ids: `os validate`, `os build` (and `os compile`, which `os dev` runs when it compiles at startup, and on each watch recompile under `--verbose`), `os lint` (as `where: message`) and the scaffold check `os init` runs print the new `message` on the text face, and `os validate --json` and `os build --json` carry it in their `errors` and author-time `issues`. `os verify` prints the error-severity ids' new text in its refusal list (`ai-skill-surface-mismatch`, `object/managed-api-method-unaffordable`, `org-axis-cross-org-bu-grant`) and counts the warnings without printing them. At the runtime publish gate (Studio, REST `/meta`, MCP): an `object` write's `object/managed-api-method-unaffordable` changes the 422 issue's `message` and the refusal log line under `OS_ALLOW_UNLINTED_METADATA_WRITES`; a `flow` write's `flow-credential-literal` and a `permission` write's `permission-retired-lifecycle-residue` change the 2xx `advisories` entry and the `[Protocol] authoring advisory` log line. Each issue's `hint` is unchanged. - **Never at the runtime gate:** `org-axis-cross-org-bu-grant`, the two seed ids, `semantic-role-field-unprovisioned` and `field-no-consumers` (CLI-only rules), and the two AI ids, whose rules run at that door only for a `flow` write, whose snapshot carries no agents or skills of their own. diff --git a/.changeset/pre/22161-lint-slice-14-one-line-fix.md b/.changeset/pre/22161-lint-slice-14-one-line-fix.md index 190a52507d7..962cabebf37 100644 --- a/.changeset/pre/22161-lint-slice-14-one-line-fix.md +++ b/.changeset/pre/22161-lint-slice-14-one-line-fix.md @@ -2,27 +2,27 @@ "@objectstack/lint": patch --- -fix(lint): 17 more author-time rule ids print a `fix:` line of one remedy sentence, and `os explain RULE_ID` carries what those lines used to explain +fix(lint): 16 more author-time rule ids print a `fix:` line of one remedy sentence, and `os explain RULE_ID` carries what those lines used to explain Clause-②: no -- **Shorter fix lines.** Each finding of these 17 rule ids now carries a `hint` (the CLI's `fix:` line, the runtime issue's `hint`) of one remedy sentence of at most 200 characters on every variant the lint suite fires (the lengths here are the `hint` alone; the CLI prints it after `fix: `). The longest of each, before → after: +- **Shorter fix lines.** Each finding of these 16 rule ids now carries a `hint` (the CLI's `fix:` line, the runtime issue's `hint`) of one remedy sentence of at most 200 characters on every variant the lint suite fires (the lengths here are the `hint` alone; the CLI prints it after `fix: `). The longest of each, before → after: - list-view search: `searchable-field-unknown` (448 → 129), `searchable-field-unprovisioned` (362 → 175) and `searchable-field-unsearchable` (328 → 139); - list-view sort: `sort-field-unprovisioned` (378 → 191), `sort-field-unsortable` (332 → 154) and `sort-field-unknown` (301 → 142); - visibility predicates: `visibility-predicate-unknown-function` (375 → 165), `visibility-bare-identifier` (221 → 102) and `visibility-predicate-syntax` (214 → 113); - - flow filters: `flow-filter-token-unknown` (427 → 182); - actions: `action-no-placement` (424 → 171); - import mappings: `mapping-target-field-unknown` (405 → 189); - dashboards: `unconsumed-widget-option` (396 → 135); - app navigation: `nav-object-ungranted` (370 → 118); - hook bodies: `hook-body-write-unprovisioned-anchor` (365 → 178), `hook-body-write-unknown-field` (275 → 110) and `hook-body-source-unparseable` (254 → 105). -- **One line, one remedy.** Where a fix line explained its remedy, the explanation moved: why a stale or inadmissible `searchableFields` entry is a `400 INVALID_FIELD` on list search, that a view narrows the object's searchable set and never widens it, and that a lookup column stores only an id; that both runtime doors refuse a formula sort with `400 INVALID_SORT`; how to vouch for an unprovisioned anchor (`columnMap`) or opt the object out of the injection (`ownership: 'none'`, `systemFields: { audit: false }`); the two vocabularies a flow filter token resolves in; the other-language operator spellings CEL does not parse, why re-spelling cannot fix an unregistered function, and which namespaces each surface binds; why gating a nav entry does not clear a missing grant; which hook-body write shapes are checked; what a partially parsed body leaves unchecked. + + A seventeenth id this change shortened, `flow-filter-token-unknown`, is retired in this same release with its rule: a flow filter operand is a value slot, and a token there is refused as `expression-invalid` by `validateStackExpressions` (its `ExprIssue`), with the CEL envelope that replaces it. +- **One line, one remedy.** Where a fix line explained its remedy, the explanation moved: why a stale or inadmissible `searchableFields` entry is a `400 INVALID_FIELD` on list search, that a view narrows the object's searchable set and never widens it, and that a lookup column stores only an id; that both runtime doors refuse a formula sort with `400 INVALID_SORT`; how to vouch for an unprovisioned anchor (`columnMap`) or opt the object out of the injection (`ownership: 'none'`, `systemFields: { audit: false }`); the other-language operator spellings CEL does not parse, why re-spelling cannot fix an unregistered function, and which namespaces each surface binds; why gating a nav entry does not clear a missing grant; which hook-body write shapes are checked; what a partially parsed body leaves unchecked. - **A list the verdict already covers is not echoed.** `searchable-field-unknown`, `sort-field-unknown` and `mapping-target-field-unknown` no longer list every field of the object: the verdict's "Did you mean" names the near miss. `mapping-target-field-unknown` still lists the object's compound fields and their parts, as every import-mapping refusal does. - **`os explain RULE_ID` carries the rest.** What the fix lines stopped saying is each id's `RULE_EXPLANATIONS` entry in `@objectstack/lint`, which `os explain RULE_ID` prints. Two ids had no entry and gain one, so their `rule:` line now ends with the `os explain` pointer: `action-no-placement` ("where an action can be placed") and `unconsumed-widget-option` ("which widget options a renderer reads"). Five entries gain text. The action locations the new entry spells out are held to the spec's `ACTION_LOCATIONS` by the rule's test. -- **At the runtime publish gate** (Studio, REST `PUT /api/v1/meta`, and the MCP and AI authors that write through that door), 10 of the 17 fire; the `hint` FROM → TO, measured at the door: +- **At the runtime publish gate** (Studio, REST `PUT /api/v1/meta`, and the MCP and AI authors that write through that door), 9 of the 16 fire; the `hint` FROM → TO, measured at the door: - a `view` write: `sort-field-unknown` (422 `issues[].hint`, a name / a dotted path 114 / 261 → 47 / 135), `sort-field-unsortable` (422, 318 → 140), `sort-field-unprovisioned` (2xx `advisories[].hint`, 363 → 176), `searchable-field-unknown` (422, a name / a dotted path 281 / 430 → 48 / 129), `searchable-field-unsearchable` (422, a formula / outside the declared set / a lookup / a json column 222 / 142 / 323 / 157 → 110 / 68 / 135 / 64), `searchable-field-unprovisioned` (2xx advisory, 361 → 174), `visibility-predicate-syntax` (422, a recognised spelling / none 106 / 214 → 106 / 107), `visibility-predicate-unknown-function` (422, 371 → 165) and `visibility-bare-identifier` (422, a bare name / an unbound namespace 160 / 215 → 41 / 96); - - an `object` write: `searchable-field-unknown` (422, the object's own set / a built-in list view 266 / 261 → 55 / 50), `searchable-field-unsearchable` (422, 222 → 110) and `searchable-field-unprovisioned` (2xx advisory, 361 → 174); - - a `flow` write: `flow-filter-token-unknown` (422, `{TOMORROW()}` / `{ROUND(x)}` / `{Math.round(x)}` / `{TODAY() - 45 - 10}` 336 / 358 / 427 / 100 → 91 / 113 / 182 / 100). + - an `object` write: `searchable-field-unknown` (422, the object's own set / a built-in list view 266 / 261 → 55 / 50), `searchable-field-unsearchable` (422, 222 → 110) and `searchable-field-unprovisioned` (2xx advisory, 361 → 174). Studio's save-advisory toast prints an advisory's `hint` after its message, so the two `*-unprovisioned` toasts shorten; Studio renders no 422 issue's `hint`. No Studio, REST or MCP reader in this repository or in objectui matches on `hint` text; a tool that did should match on `rule` and `path` instead. - **Never at the runtime gate:** `action-no-placement`, `mapping-target-field-unknown`, `unconsumed-widget-option`, `nav-object-ungranted` and the three `hook-body-*` ids run on the CLI only. diff --git a/.changeset/pre/22301-verify-evaluate-condition-door.md b/.changeset/pre/22301-verify-evaluate-condition-door.md index db0f1df6505..8e25cccc834 100644 --- a/.changeset/pre/22301-verify-evaluate-condition-door.md +++ b/.changeset/pre/22301-verify-evaluate-condition-door.md @@ -9,6 +9,6 @@ Clause-②: yes (widening) An app's tests can evaluate a flow condition on the booted stack's own evaluator through the in-process handle, so a suite no longer calls the booted kernel's `automation` service by hand for a condition's truth table. - **`stack.automation.evaluateCondition(condition, variables)`** calls the booted `automation` service's `evaluateCondition`, the method a flow's start gate, its edges and its `decision` branches are decided by. It resolves with the engine's boolean, unchanged. It is for a truth table over variable shapes no write produces. -- **`condition`** takes the shapes the engine takes, and is handed over as given: an envelope (`{ dialect: 'cel', source }`) or a string, which the engine reads as CEL unless it carries a `{var}` hole (the legacy template dialect). The engine's own flow sites hand the evaluator an envelope (a start node's string `condition` and a `decision` branch's `expression` are wrapped as `{ dialect: 'cel', source }`, and an edge's condition already is one), so pass the envelope to model any of them. +- **`condition`** takes the shapes the engine takes, and is handed over as given: an envelope (`{ dialect: 'cel', source }`) or a string, which the engine reads as CEL; a `{var}` hole is refused as the brace trap, and a `template` or `flow` envelope is refused too (the legacy template dialect is deleted in this release). The engine's own flow sites hand the evaluator an envelope (a start node's string `condition` and a `decision` branch's `expression` are wrapped as `{ dialect: 'cel', source }`, and an edge's condition already is one), so pass the envelope to model any of them. - **`variables`** is a plain object. Each own key becomes one entry of the `Map` the engine takes: `{ record, previous }` binds `record` and `previous`. - **Refusals are not the handle's.** A condition the engine cannot evaluate rejects with the engine's own error, never `false`. On a stack with no automation service it rejects with the kernel's own `SERVICE_NOT_REGISTERED` error, whose `serviceName` is `'automation'` (`isServiceNotRegisteredError` from `@objectstack/core` answers `true`). Boot with `automation: true`, or have the app declare `requires: ['automation']`. No error code is added. diff --git a/.changeset/pre/22502-flow-binding-variable-dollar-name-refused.md b/.changeset/pre/22502-flow-binding-variable-dollar-name-refused.md index 3ef96994d30..554708d637b 100644 --- a/.changeset/pre/22502-flow-binding-variable-dollar-name-refused.md +++ b/.changeset/pre/22502-flow-binding-variable-dollar-name-refused.md @@ -30,7 +30,7 @@ Clause-②: no (narrowing) **The one-line fix: name the variable without the `$`, and rename every read of it with it.** -No D2 conversion rewrites the name: the bare name may already be bound in the flow, and the reads of the old name sit in every dialect a flow string speaks (a text-slot hole, a CEL expression, a single-brace token in a value position), so the rename is the author's. +No D2 conversion rewrites the name: the bare name may already be bound in the flow, and the reads of the old name sit in every dialect a flow string speaks (a text-slot hole, a CEL expression), so the rename is the author's. **Who is affected, measured.** The last published spec, `@objectstack/spec@17.7.0` (npm `latest`), types all six keys as plain strings and accepts any `$` name. This repository was measured with `git grep` over `packages`, `examples`, `skills`, `apps`, `content` and `docs`: the only `$`-named `errorVariable` / `outputVariable` other than `$error` were three test fixtures, all renamed here (`errorVariable: '$err'` in two `packages/spec` automation tests, `errorVariable: '$caught'` in a `service-automation` test). Every authored `errorVariable` in examples and docs is `$error`, which stays legal, and no example or doc binds a `$`-named `outputVariable`. The pinned `objectui` checkout uses `errorVariable: '$error'` only. Deployed metadata and other repositories were not measured. diff --git a/.changeset/pre/22572-flow-binding-name-dollar-refused.md b/.changeset/pre/22572-flow-binding-name-dollar-refused.md index 90723dcd1e4..9262f9bd743 100644 --- a/.changeset/pre/22572-flow-binding-name-dollar-refused.md +++ b/.changeset/pre/22572-flow-binding-name-dollar-refused.md @@ -38,7 +38,7 @@ Clause-②: no (narrowing) **The one-line fix: name the variable without the `$`, and rename every read of it with it.** -No D2 conversion rewrites the name: the bare name may already be bound in the flow, and the reads of the old name sit in every dialect a flow string speaks (a text-slot hole, a CEL expression, a single-brace token in a value position), so the rename is the author's. +No D2 conversion rewrites the name: the bare name may already be bound in the flow, and the reads of the old name sit in every dialect a flow string speaks (a text-slot hole, a CEL expression), so the rename is the author's. **Who is affected, measured.** The last published spec, `@objectstack/spec@17.7.0` (npm `latest`), types every one of these keys as a plain string. Measured on `main` at this change's base: the 35 flows `examples/app-crm`, `examples/app-todo` and `examples/app-showcase` ship all parse clean under the new rule, and a TypeScript-AST scan of `examples` (234 files), `packages/platform-objects` (167, which ships no flow), `packages/qa/dogfood` (280), the rest of `packages` and the `hotcrm` app found no `$`-led binding on any of these positions. The pinned `objectui` checkout binds none either. Deployed metadata and other repositories were not measured. diff --git a/content/docs/automation/flows.mdx b/content/docs/automation/flows.mdx index f9e4b12f869..3a5c70f8a93 100644 --- a/content/docs/automation/flows.mdx +++ b/content/docs/automation/flows.mdx @@ -630,8 +630,8 @@ later node reads `{subject}`, not `{screen_1.subject}`. } ``` -`visibleWhen` is **bare CEL over the screen's own field names** — not the `{var}` -template dialect the rest of a node's `config` uses. The client re-evaluates it +`visibleWhen` is **bare CEL over the screen's own field names** — never a +`{{ }}` template like the screen's `title` / `description`. The client re-evaluates it as the user types, against the values collected so far, which is why it cannot be a server-interpolated template. Omit it for a field that is always visible. @@ -650,7 +650,7 @@ Two things worth knowing: the name is never bound and a downstream condition reading it aborts the run. - **A broken predicate fails open** (the field stays visible) rather than hiding an input the flow may be waiting on. `registerFlow()` does check the predicate - as bare CEL — a `{var}` template or a syntax error is a loud registration + as bare CEL — a `{…}` brace or a syntax error is a loud registration failure that quotes the source and locates it as `config.fields[N].visibleWhen` — but a predicate that *parses* and simply names the wrong field still shows up as a field that never hides. @@ -2202,19 +2202,14 @@ a field access on an object variable is not one. Both reported `success`. They are bare CEL now, so the spellings in the table above are the correct ones and `lead_record.status == 'converted'` resolves the field. -The `{var}` form still works where a condition is a plain authored string — a -start node's `config.condition`: `{amount} > 100`, `{status} == 'active'`. The -two ways it used to answer `false` without saying so are now **loud errors** -naming the reference: a `{…}` hole that matches no flow variable, and a -substituted value that is neither a boolean, a number, nor part of a comparison -(#4336). - -**A decision's `conditions[].expression` is the exception — it is always CEL.** -The slot is declared bare CEL and is on the expression ledger as a predicate -(#4439), so a braced spelling there is not the `{var}` dialect but a build -failure: `os build` and `registerFlow` reject it, naming -`config.conditions[N].expression`. That is deliberate — the alternative is a -build that refuses what run time would happily execute. +**There is no `{var}` form of a condition.** Every condition is bare CEL — a +start node's `config.condition`, an edge's `condition`, a decision's +`conditions[].expression`, a screen field's `visibleWhen` — so a braced +spelling (`{amount} > 100`) is a build failure: `os build` and `registerFlow` +reject it, naming the slot (`config.conditions[N].expression`, …). Write +`amount > 100`, `status == 'active'`. The template path the evaluator kept for a +braced string is deleted (#19939), so a direct `evaluateCondition` call refuses +the same spelling as a CEL fault, and a `template` or `flow` envelope with it. CEL conditions that fail to evaluate raise an error and stop the run — they diff --git a/content/docs/references/automation/builtin-node-config.mdx b/content/docs/references/automation/builtin-node-config.mdx index abd2519303d..712bc983126 100644 --- a/content/docs/references/automation/builtin-node-config.mdx +++ b/content/docs/references/automation/builtin-node-config.mdx @@ -36,14 +36,14 @@ ADR-0087 D2 conversion layer like `notify.source` before it. Live execute-time contracts: each executor `parse()`s its config against its schema before running (`service-automation`'s `parse-config.ts`), so type and `required` violations refuse the node as a guard. All of these -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), 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`). +parse the RAW stored config, as authored: no executor interpolates one — +the single-brace `{token}` dialect is retired from every flow position and +its interpolator deleted (#19939, ADR-0032 Sequencing 6). The value slots +refuse a `{token}` (the "value slots" section below), each operand of the +CRUD `filter` included, where a query placeholder stays the query engine's; +and 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`). ## Unknown keys — closed here too, as of #4001 批 9 diff --git a/content/docs/references/data/context-tokens.mdx b/content/docs/references/data/context-tokens.mdx index 6ab90d4a8df..c804a0f30d9 100644 --- a/content/docs/references/data/context-tokens.mdx +++ b/content/docs/references/data/context-tokens.mdx @@ -110,10 +110,10 @@ Three spellings look alike and are three different mechanisms: - `{record_id}` — this filter token. A whole filter VALUE, resolved to the id of the record the page shows. -- `{recordId}` — a URL / flow-template placeholder: an action's - `newTabUrl` (`action.zod.ts`) and the flow template dialect - (`@objectstack/lint`'s `flow-template-grammar.ts`). It is interpolated by - those surfaces' own template engines and is NOT a filter token. Written +- `{recordId}` — a URL placeholder: an action's `newTabUrl` + (`action.zod.ts`), interpolated by that surface's own template engine, + and a variable's spelling in the retired flow template dialect (refused + at every flow position since #19939). It is NOT a filter token. Written inside a filter it is refused, with `{record_id}` suggested. - `'record_id'` — a page-variable TYPE (`PageVariableSchema.type` in `page.zod.ts`), naming what a page variable holds, e.g. the selection of diff --git a/packages/formula/src/stdlib.ts b/packages/formula/src/stdlib.ts index ecf451d4074..7257b407c84 100644 --- a/packages/formula/src/stdlib.ts +++ b/packages/formula/src/stdlib.ts @@ -91,9 +91,9 @@ const MAX_TIMESTAMP_MS = 253_402_300_799_999; /** * The ISO text of a timestamp: `toISOString()`, the UTC calendar, three-digit - * milliseconds and a `Z`. These are the bytes the flow template dialect writes - * for `{NOW()}`, and their first ten characters are what it writes for - * `{TODAY()}` (`service-automation` `builtin/template.ts`, `resolveToken`). + * milliseconds and a `Z`. These are the bytes the retired flow template + * dialect wrote for `{NOW()}`, and their first ten characters are what it + * wrote for `{TODAY()}` (the 17.x `resolveToken`, deleted in #19939). * * An invalid or out-of-range instant is refused rather than rendered. The * refusal is a throw, which the engine reports as a runtime error. @@ -364,7 +364,7 @@ export function registerStdLib( // The string form of a timestamp, the reverse of `date` / `datetime`: // `isoDate(t)` is `YYYY-MM-DD` and `isoDatetime(t)` is // `YYYY-MM-DDTHH:mm:ss.sssZ`, both on the UTC calendar. They write the - // bytes the flow template dialect writes for `{TODAY()}` and `{NOW()}`. + // bytes the retired flow template dialect wrote for `{TODAY()}` and `{NOW()}`. // `isoDate(today())` is the reference-timezone day, because `today()` is // that day at UTC midnight (ADR-0053 D1). // diff --git a/packages/lint/src/flow-template-grammar.test.ts b/packages/lint/src/flow-template-grammar.test.ts deleted file mode 100644 index fe7c348a1d1..00000000000 --- a/packages/lint/src/flow-template-grammar.test.ts +++ /dev/null @@ -1,124 +0,0 @@ -// Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license. -// -// The CROSS-PACKAGE DRIFT PIN for `flow-template-grammar.ts` (#16096). -// -// That module MIRRORS the automation template evaluator's whole-token dispatch, -// because `@objectstack/lint` depends on `@objectstack/spec` and never on a -// runtime, so the dialect cannot be imported from the package that owns it. A -// mirror nobody checks is the "N copies, the next author fixes one of N" shape -// `filter-walk.ts` was written against — so this file reads the ORIGINAL from -// disk and fails when any mirrored piece stops matching it. -// -// The read escapes this package, spelled so `check:cross-package-test-inputs` -// can see it, and `$TURBO_ROOT$/packages/services/service-automation/src/**` is -// already a declared input of `@objectstack/lint#test` in turbo.json. - -import { existsSync, readFileSync } from 'node:fs'; -import { dirname, join } from 'node:path'; -import { fileURLToPath } from 'node:url'; -import { describe, expect, it } from 'vitest'; - -import { - classifyFlowTemplateToken, - DATE_FUNCTION_RE, - VARIABLE_PATH_RE, - SAFE_EXPRESSION_RE, - IDENTIFIER_SCAN_RE, - CALL_POSITION_RE, - FLOW_TEMPLATE_DATE_FUNCTIONS, - FLOW_TEMPLATE_VALUE_FUNCTIONS, -} from './flow-template-grammar.js'; - -const HERE = dirname(fileURLToPath(import.meta.url)); - -/** Walk up to the workspace root — the directory holding pnpm-workspace.yaml. */ -function findUp(predicate: (dir: string) => boolean): string { - let dir = HERE; - for (;;) { - if (predicate(dir)) return dir; - const parent = dirname(dir); - if (parent === dir) throw new Error('workspace root not found from ' + HERE); - dir = parent; - } -} -const REPO = findUp((dir) => existsSync(join(dir, 'pnpm-workspace.yaml'))); - -const ORIGINAL = join(REPO, 'packages/services/service-automation/src/builtin/template.ts'); -// Loud absence: if this file moves, the mirror is unpinned, and an unpinned -// mirror is the defect this test exists to prevent. Failing to read IS the -// regression — never a skip. -const source = readFileSync(ORIGINAL, 'utf8'); - -describe('the mirrored grammar still matches the evaluator that owns it', () => { - const mirrored: Array<[string, RegExp]> = [ - ['the NOW()/TODAY() ± N day form', DATE_FUNCTION_RE], - ['the variable / dotted-path form', VARIABLE_PATH_RE], - ['the arithmetic character set', SAFE_EXPRESSION_RE], - ['the identifier scan', IDENTIFIER_SCAN_RE], - ['the call-position lookahead', CALL_POSITION_RE], - ]; - for (const [label, re] of mirrored) { - it(`${label} appears verbatim in template.ts`, () => { - expect(source).toContain(re.source); - }); - } - - it('mirrors the value-function table exactly — no name added, none dropped', () => { - const block = /const EXPRESSION_FUNCTION_ARITY[^{]*\{([\s\S]*?)\n\};/.exec(source); - expect(block, 'EXPRESSION_FUNCTION_ARITY not found in template.ts').toBeTruthy(); - const names = [...block![1].matchAll(/^\s*([A-Za-z_$][\w$]*)\s*:/gm)].map((m) => m[1]); - expect(names.sort()).toEqual([...FLOW_TEMPLATE_VALUE_FUNCTIONS].sort()); - }); - - it('mirrors the two whole-token date function names', () => { - for (const name of FLOW_TEMPLATE_DATE_FUNCTIONS) expect(DATE_FUNCTION_RE.source).toContain(name); - // And the evaluator still keeps them OUT of the value table — the reason - // `{TODAY() - 45 - 10}` is refused while `{TODAY() - 45}` is not. - const block = /const EXPRESSION_FUNCTION_ARITY[^{]*\{([\s\S]*?)\n\};/.exec(source); - for (const name of FLOW_TEMPLATE_DATE_FUNCTIONS) expect(block![1]).not.toContain(name); - }); - - it('the evaluator still REFUSES an unknown call rather than resolving it to null', () => { - // The mirror only means something while the runtime still throws here. - expect(source).toContain('throw unknownFunctionError(match, trimmed)'); - }); - - // [#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'); - }); -}); - -describe('dispatch ORDER — the property the negative control depends on', () => { - it('classifies {TODAY() - 45} as a date function, never as a call', () => { - expect(classifyFlowTemplateToken('TODAY() - 45')).toEqual({ kind: 'date-function', name: 'TODAY' }); - }); - - it('classifies TOMORROW() as an unknown function', () => { - expect(classifyFlowTemplateToken('TOMORROW()')).toEqual({ kind: 'unknown-function', name: 'TOMORROW' }); - }); - - it('classifies the open arm as variable-path, never as a finding', () => { - expect(classifyFlowTemplateToken('recordId')).toEqual({ kind: 'variable-path', head: 'recordId' }); - expect(classifyFlowTemplateToken('record.id')).toEqual({ kind: 'variable-path', head: 'record' }); - }); - - it('classifies $User.* as user context', () => { - expect(classifyFlowTemplateToken('$User.Id')).toEqual({ kind: 'user-context' }); - }); - - it('classifies a junk shape as unresolvable rather than as a call', () => { - expect(classifyFlowTemplateToken('30 days ago')).toEqual({ kind: 'unresolvable-shape' }); - expect(classifyFlowTemplateToken('')).toEqual({ kind: 'unresolvable-shape' }); - }); - - it('never reports a reserved literal in call position', () => { - expect(classifyFlowTemplateToken('null(1)').kind).not.toBe('unknown-function'); - }); -}); diff --git a/packages/lint/src/flow-template-grammar.ts b/packages/lint/src/flow-template-grammar.ts deleted file mode 100644 index 031bae79a66..00000000000 --- a/packages/lint/src/flow-template-grammar.ts +++ /dev/null @@ -1,160 +0,0 @@ -// Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license. - -/** - * @module flow-template-grammar - * - * **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. - * - * 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). - * - * 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. - * - `round` / `floor` / `ceil` / `abs` / `min` / `max` in CALL position — - * closed by the maintainer ruling commit 815585513 records ("exactly … every name and semantic - * mirrored **1:1 from the CEL stdlib**, ⛔ no second semantics invented"). - * - a bare or dotted identifier (`{recordId}`, `{record.id}`, `{status}`) — - * **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. - * - * ## Dispatch ORDER is load-bearing, not incidental - * - * `resolveToken` tries the date-function form BEFORE it scans for call - * positions. `{TODAY() - 45}` therefore never reaches the scan — which is the - * only reason the legitimate spelling stays silent, because `TODAY` sitting in - * front of a `(` is otherwise indistinguishable from `TOMORROW`. This module - * mirrors that order exactly and `flow-template-grammar.test.ts` pins the - * negative control against it. - * - * ## This is a MIRROR, and the drift is pinned - * - * `@objectstack/lint` depends on `@objectstack/spec` and never on a runtime - * (its own package description), so the dialect cannot be imported from the - * package that owns it. The five regexes and the function table below are - * therefore copied, and `flow-template-grammar.test.ts` reads - * `packages/services/service-automation/src/builtin/template.ts` from disk and - * fails when any of them stops matching the original — a cross-package test - * input already declared on `@objectstack/lint#test` in `turbo.json`, so the - * graph can see it. ⛔ Do not "simplify" a regex here: it is not this module's - * to choose, and an equivalent-looking rewrite breaks the pin that keeps the - * two readers honest. - */ - -/** - * The two whole-token date functions, with their `± N day` offset grammar. - * Verbatim from `resolveToken`'s `dateFnMatch`. - */ -export const DATE_FUNCTION_RE = /^(NOW|TODAY)\s*\(\s*\)\s*(?:([+\-])\s*(\S+))?$/; - -/** Direct variable / dotted-path lookup, numeric segments included (#1872). */ -export const VARIABLE_PATH_RE = /^[A-Za-z_$][\w$]*(?:\.(?:[A-Za-z_$][\w$]*|\d+))*$/; - -/** - * The character set `resolveToken` will attempt arithmetic on. A token outside - * it resolves to `undefined` without ever reaching the call-position scan. - */ -export const SAFE_EXPRESSION_RE = /^[\w\s+\-*/%().,?:<>=!&|"'$]+$/; - -/** Identifier / dotted-identifier occurrences inside a mixed expression. */ -export const IDENTIFIER_SCAN_RE = /([A-Za-z_$][\w$]*(?:\.[A-Za-z_$][\w$]*)*)/g; - -/** An identifier is in CALL position when a `(` follows it. */ -export const CALL_POSITION_RE = /^\s*\(/; - -/** Literals `resolveToken` never substitutes, checked BEFORE call position. */ -const RESERVED_LITERALS: ReadonlySet = new Set(['true', 'false', 'null', 'undefined']); - -/** The two names legal only as a whole token (`{TODAY() + 7}`), never in a call. */ -export const FLOW_TEMPLATE_DATE_FUNCTIONS: readonly string[] = ['NOW', 'TODAY']; - -/** - * The value-expression function table — the CEL stdlib's numeric six, by the - * ruling commit 815585513 records. Mirrors `EXPRESSION_FUNCTION_ARITY`'s key set. - */ -export const FLOW_TEMPLATE_VALUE_FUNCTIONS: readonly string[] = [ - 'round', 'floor', 'ceil', 'abs', 'min', 'max', -]; - -const VALUE_FUNCTION_SET: ReadonlySet = new Set(FLOW_TEMPLATE_VALUE_FUNCTIONS); - -/** What the flow template dialect does with one whole-string `{…}` token. */ -export type FlowTemplateTokenVerdict = - /** `{NOW()}` / `{TODAY() - 45}` — the evaluator resolves it. Legitimate. */ - | { kind: 'date-function'; name: string } - /** `{$User.Id}` — the evaluator resolves it from the run context. */ - | { kind: 'user-context' } - /** - * `{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). - */ - | { kind: 'unknown-function'; name: string } - /** - * Anything else — junk shapes (`{30 days ago}`) and arithmetic over names - * this module cannot resolve. `resolveToken` answers `undefined`. - */ - | { kind: 'unresolvable-shape' }; - -/** - * 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 - * severity and knows nothing about where the token was found. - */ -export function classifyFlowTemplateToken(inner: string): FlowTemplateTokenVerdict { - const trimmed = inner.trim(); - if (!trimmed) return { kind: 'unresolvable-shape' }; - - // 1. Whole-token date functions, BEFORE any call-position reasoning. - const dateMatch = DATE_FUNCTION_RE.exec(trimmed); - if (dateMatch) return { kind: 'date-function', name: dateMatch[1] }; - - // 2. `$User.*` shortcuts. - if (trimmed.startsWith('$User.')) return { kind: 'user-context' }; - - // 3. Direct variable / dotted path — the open arm. - if (VARIABLE_PATH_RE.test(trimmed)) { - return { kind: 'variable-path', head: trimmed.split('.')[0] }; - } - - // 4. Outside the arithmetic character set: `undefined`, no throw. - if (!SAFE_EXPRESSION_RE.test(trimmed)) return { kind: 'unresolvable-shape' }; - - // 5. The call-position scan. `resolveToken` throws on the FIRST unknown name - // it reaches, so the first is what an author sees and what is reported. - for (const match of trimmed.matchAll(IDENTIFIER_SCAN_RE)) { - const name = match[0]; - if (RESERVED_LITERALS.has(name)) continue; - const rest = trimmed.slice((match.index ?? 0) + name.length); - if (!CALL_POSITION_RE.test(rest)) continue; - if (VALUE_FUNCTION_SET.has(name)) continue; - return { kind: 'unknown-function', name }; - } - - return { kind: 'unresolvable-shape' }; -} diff --git a/packages/lint/src/index.ts b/packages/lint/src/index.ts index a9154f5def1..acec2cdb975 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 { 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'; export { validatePresetComparands, FILTER_PRESET_COMPARAND } from './validate-preset-comparands.js'; export type { PresetComparandFinding, PresetComparandSeverity } from './validate-preset-comparands.js'; diff --git a/packages/lint/src/lint-flow-patterns.test.ts b/packages/lint/src/lint-flow-patterns.test.ts index 097b6ec4c55..6924c22d0d2 100644 --- a/packages/lint/src/lint-flow-patterns.test.ts +++ b/packages/lint/src/lint-flow-patterns.test.ts @@ -2803,9 +2803,11 @@ describe('#16405 — an `http` node payload is not a region, and both #1315 rule // [#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', () => { + // envelope and the filter operand's remedy. [#19939 S7] At a position no + // slot judges (a plugin node's own config) nothing resolves a reference + // since the single-brace interpolator is deleted, so the hint says the + // value is read as written and offers no `{var}` token. + it('control: a filter operand names its envelope now; outside every slot the hint says the value is read as written', () => { const filtered = (owner: string) => valueFlow('get_record', { objectName: 'task', filter: { owner } }); const [f] = bareDollar(filtered('$source.id')); expect(f!.message).toContain('in the get_record filter operand'); @@ -2815,7 +2817,9 @@ describe('#16405 — an `http` node payload is not a region, and both #1315 rule 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}`'); + expect(plugin!.message).toContain('this flow node value is read as written'); + expect(plugin!.hint).toContain('neither `{var}` nor `{{ }}` resolves here'); + expect(plugin!.hint).not.toContain('`{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'); @@ -2826,8 +2830,8 @@ describe('#16405 — an `http` node payload is not a region, and both #1315 rule // hint said `Use {var} (e.g. {record.title})` in a value slot too — a // spelling the value-slot judge refuses there. In a value slot it now names // the CEL envelope the judge writes for the string with each `{{ }}` hole - // read as the token it means, asked at the string's position; elsewhere the - // single brace still resolves and the hint is unchanged. + // read as the token it means, asked at the string's position. (Elsewhere, + // since #19939 S7, the hint says the value is read as written.) describe('a double brace in a value slot', () => { // [#22677] The prescriptions read `record.title`, so the flow is handed a // record (a record trigger on a declared `task`): with no entrance the @@ -2919,13 +2923,15 @@ describe('#16405 — an `http` node payload is not a region, and both #1315 rule // [#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', () => { + it('control: a filter operand names its envelope now; outside every slot the double-brace hint says the value is read as written', () => { const [f] = doubleBrace(valueFlow('get_record', { objectName: 'task', filter: { title: '{{ 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}`)'); + expect(plugin!.message).toContain('nothing renders a template here'); + expect(plugin!.hint).toContain('neither `{var}` nor `{{ }}` resolves here'); + expect(plugin!.hint).not.toContain('Use `{var}`'); // …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 64944afc2f0..c0a01e51da6 100644 --- a/packages/lint/src/lint-flow-patterns.ts +++ b/packages/lint/src/lint-flow-patterns.ts @@ -171,9 +171,6 @@ import { DecisionConfigSchema } from '@objectstack/spec/automation'; // driver-sql, driver-mongodb and driver-memory execute. This linter asks it // rather than hand-writing a fourth copy; see {@link filterCarriesNoCondition}. import { reduceFilterVerdict } from '@objectstack/spec/data'; -// [#19939] The lint's pinned mirror of the 17.x interpolator's token dispatch — -// here only to tell a bare run-user reference (`$User.Id`) from a variable. -import { classifyFlowTemplateToken } from './flow-template-grammar.js'; import { stripRegions, ownRegionKeys, REGION_SLOTS, MAX_REGION_DEPTH } from './flow-walk.js'; import { recordsOf } from './object-graph.js'; @@ -667,15 +664,16 @@ function scanFilterForDateEquality( // `actionUrl`, an http `url` and each `headers` value, a screen `title` / // `description`, an `end` `message` — `FLOW_NODE_TEXT_SLOTS`): they render // the `{{ }}` holes of the formula template dialect; -// - every other config string (a `filter`, …): it keeps the SINGLE-brace -// dialect (`{var}` / `{rec.field}` / `{$User.Id}`) — except a node's -// literals, read as written (#19939: a notify node's `sourceObject`, +// - every other config string (a plugin node's own config, a +// `connector_action` input, …): read as written — no flow template dialect +// resolves there (#19939 S7 deleted the single-brace interpolator). +// A node's declared literals (#19939: a notify node's `sourceObject`, // `actorId` and `payload`; an http node's `method`, `durable`, `timeoutMs` -// and `signingSecret`), where neither dialect resolves -// ({@link LITERAL_KEYS}). +// and `signingSecret`, {@link LITERAL_KEYS}) are the same reading, named +// key by key, and the contract refuses a token there. // Two wrong-syntax mistakes AI/human authors carry over between them, or from // other platforms: -// - `{{ai_reply}}` — double-brace on a single-brace value. NOT flagged on a +// - `{{ai_reply}}` — double-brace outside a text slot. NOT flagged on a // text slot, where it is the spelling; the reverse // mistake there — a single-brace token on a text slot — // is refused at `error` by the build door @@ -684,8 +682,8 @@ function scanFilterForDateEquality( // - `$source.id` — a `$`-prefixed reference written bare (resolves as a // literal string). Its hint names what the judge of the // slot it sits in accepts: the CEL envelope in a value -// slot, the `{{ }}` hole in a text slot, `{source.id}` -// elsewhere. +// slot, the `{{ }}` hole in a text slot, and elsewhere +// that the value is read as written. const DOUBLE_BRACE = /\{\{\s*[\w$][\w$.\s]*\}\}/; // A `$Ident.field` not immediately inside a `{` (so `{$User.Id}` is NOT flagged). // Require a letter/_ after `$` so currency like `$5.00` is never matched. @@ -699,6 +697,17 @@ function bareDollarRefsOf(text: string): string[] { return [...new Set([...text.matchAll(BARE_DOLLAR_REFS)].map((m) => m[1]!))]; } +/** + * Whether `ref` is a run-user reference (`$User.Id`, `$User.`) — the + * retired dialect's `$User.` shortcut, whose value-slot remedy is the CEL + * scope's `current_user` (the spec's value-slot judge names it). The lint's + * mirror of that dialect's grammar went with its interpolator (#19939 S7); + * this is the one arm of it the hint below reads. + */ +function isRunUserRef(ref: string): boolean { + return ref.startsWith('$User.'); +} + /** * Whether the flow engine binds the `$` root of `ref` (`$error`, `$record`, …). * Asked of the spec's text-slot judge, which admits a `{{ }}` hole over a `$` @@ -784,7 +793,7 @@ function valueSlotBareDollarHint(text: string, at: ValueSlotPosition): string { + 'text it spells, so a bare `$` reference is stored as written.', ]; for (const ref of bareDollarRefsOf(text)) { - const ownVariable = classifyFlowTemplateToken(ref).kind !== 'user-context' && !engineBindsRootOf(ref); + const ownVariable = !isRunUserRef(ref) && !engineBindsRootOf(ref); const root = ref.split('.')[0]!; const token = ownVariable ? `{${ref.slice(1)}}` : `{${ref}}`; parts.push( @@ -840,6 +849,18 @@ const LITERAL_KEYS: ReadonlyMap> = new Map([ ['http', new Set(['method', 'durable', 'timeoutMs', 'signingSecret'])], ]); +/** + * [#19939 S7] The hint of either finding at a config string outside every value + * slot, text slot and declared literal — a plugin node's own config, a + * `connector_action` input, … No executor renders a template there, and the + * single-brace interpolator is deleted, so nothing resolves a reference in + * either dialect: the hint names the value itself — never a `{var}` token, + * which nothing reads. + */ +const ELSEWHERE_HINT = + 'This flow node value is read as written: neither `{var}` nor `{{ }}` resolves here, and nothing evaluates a ' + + 'reference, braced or bare. Write the value itself.'; + /** The hint of either finding at a node's literal, by node type: nothing resolves a reference there. */ const LITERAL_HINTS: ReadonlyMap = new Map([ [ @@ -869,7 +890,7 @@ function literalAt(nodeType: string, path: readonly (string | number)[]): { at: return { at, hint }; } -/** `config` without its node type's `{{ }}` text slots — the strings that keep the single-brace dialect. */ +/** `config` without its node type's `{{ }}` text slots — the strings no template renders. */ function withoutTextSlots(nodeType: unknown, config: unknown): unknown { if (!config || typeof config !== 'object' || Array.isArray(config)) return config; const keys = FLOW_NODE_TEXT_SLOTS.filter((slot) => slot.nodeType === nodeType).map((slot) => slot.key); @@ -1922,9 +1943,9 @@ export function lintFlowPatterns(stack: AnyRec): FlowLintFinding[] { // top-level list did (#16910). Belt and braces, ⛔ not one repair twice. const graphEdges = recordsOf(graph.edges); - // (b) #1315 — wrong interpolation syntax in any node's template values. Flow - // node values use SINGLE braces; double-brace `{{ }}` and bare `$ref.x` - // are carried over from the formula template dialect / other platforms. + // (b) #1315 — wrong interpolation syntax in any node's values. Double-brace + // `{{ }}` outside a text slot and a bare `$ref.x` are carried over from + // the formula template dialect / other platforms. for (const node of graphNodes) { const nodeWhere = `${at} · node '${node.id}' (${node.type})`; @@ -1984,7 +2005,8 @@ export function lintFlowPatterns(stack: AnyRec): FlowLintFinding[] { for (const { value: str, path } of strings) { // [#19939] In a value slot both hints name the CEL envelope the // value-slot judge accepts, asked at the string's own position; - // everywhere else the single brace still resolves, and they name it. + // everywhere else nothing resolves a reference (#19939 S7 deleted + // the single-brace interpolator), and they say so. const at: ValueSlotPosition = { nodeType: String(node.type), config: templateConfig, path }; const valueSlot = DOUBLE_BRACE.test(str) || BARE_DOLLAR_REF.test(str) ? valueSlotRefusalAt(at.nodeType, at.config, at.path)?.label @@ -2016,11 +2038,10 @@ export function lintFlowPatterns(stack: AnyRec): FlowLintFinding[] { findings.push({ where: nodeWhere, message: valueSlot === undefined - ? `double-brace interpolation \`${str.trim().slice(0, 80)}\` — this flow node value uses SINGLE braces.` + ? `double-brace interpolation \`${str.trim().slice(0, 80)}\` — nothing renders a template here: this flow node value is read as written.` : `double-brace interpolation \`${str.trim().slice(0, 80)}\` — the ${valueSlot} reads a CEL value envelope, not a template.`, hint: valueSlot === undefined - ? `Use \`{var}\` (e.g. \`{record.title}\`): this flow node value is a string template in which only ` + - `single-brace \`{…}\` tokens resolve and all other text is literal. Double-brace \`{{ }}\` is ` + + ? `${ELSEWHERE_HINT} Double-brace \`{{ }}\` is ` + `the template dialect of the text slots only — a notify \`title\` / \`message\` / \`actionUrl\`, an http \`url\` and \`headers\` values, a screen ` + `\`title\` / \`description\`, an \`end\` \`message\`.` : valueSlotDoubleBraceHint(str, at), @@ -2031,12 +2052,10 @@ export function lintFlowPatterns(stack: AnyRec): FlowLintFinding[] { findings.push({ where: nodeWhere, message: valueSlot === undefined - ? `\`${str.trim().slice(0, 80)}\` looks like a reference written as a literal — a bare \`$ref.field\` is NOT interpolated.` + ? `\`${str.trim().slice(0, 80)}\` looks like a reference written as a literal — a bare \`$ref.field\` is NOT resolved: this flow node value is read as written.` : `\`${str.trim().slice(0, 80)}\` looks like a reference written as a literal — a bare \`$ref.field\` in the ${valueSlot} is NOT evaluated.`, hint: valueSlot === undefined - ? `Wrap it and bind a variable: \`{source.id}\` (or \`{$User.Id}\` for the current user) — a flow ` + - `node value is a string template in which only single-brace \`{…}\` tokens resolve and all ` + - `other text is literal.` + ? ELSEWHERE_HINT : valueSlotBareDollarHint(str, at), rule: FLOW_BARE_DOLLAR_REF, }); diff --git a/packages/lint/src/reference-integrity-suite.test.ts b/packages/lint/src/reference-integrity-suite.test.ts index 47730e5d39b..0c5941c0071 100644 --- a/packages/lint/src/reference-integrity-suite.test.ts +++ b/packages/lint/src/reference-integrity-suite.test.ts @@ -319,15 +319,14 @@ describe('reference-integrity suite — every member actually runs', () => { runAs: 'user', nodes: [ { id: 'start', type: 'start', config: { objectName: 'crm_lead', triggerType: 'record-after-create' } }, - // validateFlowTemplatePaths: `budget` is not a field on crm_lead. In a - // FILTER position an erased condition widens the query rather than - // narrowing it, so the runtime refuses the node — gating, not advisory - // (#3810). This member is the suite's only source of `error` findings - // from a flow, so it also pins that both severities survive the suite. + // validateFlowTemplatePaths: `budget` is not a field on crm_lead, so + // the text slot's hole renders a blank on every run — advisory. (A + // CRUD filter operand is a value slot since #19939, which this rule + // no longer scans: its `{…}` token is the value-slot judge's refusal.) { - id: 'fetch', - type: 'get_record', - config: { objectName: 'crm_lead', filter: { name: '{record.budget}' } }, + id: 'notice', + type: 'notify', + config: { title: 'Budget: {{ record.budget }}', message: 'x' }, }, // validateFlowNodeWrites: the node's structural write map names a // field crm_lead does not declare — the third surface in the #4271 @@ -406,12 +405,13 @@ describe('reference-integrity suite — every member actually runs', () => { expect(sort?.path).toBe('objects[0].listViews.aging.sort[0]'); }); - it('carries a gating flow-template finding through the suite (#3810)', () => { + it('carries the flow-template finding through the suite at its own severity', () => { const findings = validateReferenceIntegrity(stack); const flow = findings.find((f) => f.rule === 'flow-template-unknown-field'); - // A filter-position miss must reach the CLI as an ERROR, or `os lint` and - // `os compile` would print a yellow line and ship a flow that cannot run. - expect(flow?.severity).toBe('error'); + // A blank in a rendered text slot: advisory. (Its filter-position ERROR + // went with the dialect that read a token there, #19939.) + expect(flow?.severity).toBe('warning'); + expect(flow?.message).toContain("'{{ record.budget }}'"); }); it('concatenates in list order and carries the common finding shape', () => { diff --git a/packages/lint/src/validate-flow-template-paths.test.ts b/packages/lint/src/validate-flow-template-paths.test.ts index c36a5af9edb..98068755b05 100644 --- a/packages/lint/src/validate-flow-template-paths.test.ts +++ b/packages/lint/src/validate-flow-template-paths.test.ts @@ -1,6 +1,7 @@ // Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license. import { describe, it, expect } from 'vitest'; +import { flowNodeValueTemplateRefusals } from '@objectstack/spec/automation'; import { validateFlowTemplatePaths, FLOW_TEMPLATE_UNKNOWN_FIELD, @@ -42,9 +43,9 @@ function flowWith(notify: AnyRec, objectName = 'crm_lead'): AnyRec { } describe('validateFlowTemplatePaths', () => { - it('flags an unknown field in a {record.} template (typo)', () => { + it('flags an unknown field in a {{ record. }} template (typo)', () => { const findings = validateFlowTemplatePaths( - flowWith({ title: 'New lead: {record.full_naem}', body: 'x' }), + flowWith({ title: 'New lead: {{ record.full_naem }}', body: 'x' }), ); expect(findings).toHaveLength(1); expect(findings[0].rule).toBe(FLOW_TEMPLATE_UNKNOWN_FIELD); @@ -52,9 +53,9 @@ describe('validateFlowTemplatePaths', () => { expect(findings[0].message).toContain('full_naem'); }); - it('flags a lookup cross-object hop {record..}', () => { + it('flags a lookup cross-object hop {{ record.. }}', () => { const findings = validateFlowTemplatePaths( - flowWith({ title: 'From {record.crm_account.name}', body: 'x' }), + flowWith({ title: 'From {{ record.crm_account.name }}', body: 'x' }), ); expect(findings).toHaveLength(1); expect(findings[0].rule).toBe(FLOW_TEMPLATE_LOOKUP_TRAVERSAL); @@ -63,49 +64,49 @@ describe('validateFlowTemplatePaths', () => { it('does NOT flag a formula field (valid, hydrated since #3445)', () => { const findings = validateFlowTemplatePaths( - flowWith({ title: 'New lead: {record.full_name}', body: '{record.company}' }), + flowWith({ title: 'New lead: {{ record.full_name }}', body: '{{ record.company }}' }), ); expect(findings).toHaveLength(0); }); it('does NOT flag a plain scalar field', () => { const findings = validateFlowTemplatePaths( - flowWith({ title: '{record.first_name} {record.last_name}', body: '{record.company}' }), + flowWith({ title: '{{ record.first_name }} {{ record.last_name }}', body: '{{ record.company }}' }), ); expect(findings).toHaveLength(0); }); it('does NOT flag a bare lookup id (no sub-path)', () => { const findings = validateFlowTemplatePaths( - flowWith({ title: 'acct {record.crm_account}', body: 'x' }), + flowWith({ title: 'acct {{ record.crm_account }}', body: 'x' }), ); expect(findings).toHaveLength(0); }); it('does NOT flag a numeric index into a multiple lookup (#1872)', () => { const findings = validateFlowTemplatePaths( - flowWith({ title: 'ch {record.target_channels.0}', body: 'x' }), + flowWith({ title: 'ch {{ record.target_channels.0 }}', body: 'x' }), ); expect(findings).toHaveLength(0); }); it('does NOT flag a sub-path into a json field', () => { const findings = validateFlowTemplatePaths( - flowWith({ title: '{record.payload.foo}', body: 'x' }), + flowWith({ title: '{{ record.payload.foo }}', body: 'x' }), ); expect(findings).toHaveLength(0); }); it('does NOT flag system/audit columns', () => { const findings = validateFlowTemplatePaths( - flowWith({ title: '{record.id} {record.created_at} {record.owner}', body: 'x' }), + flowWith({ title: '{{ record.id }} {{ record.created_at }} {{ record.owner }}', body: 'x' }), ); expect(findings).toHaveLength(0); }); it('ignores non-record tokens (flow vars, NOW(), $User)', () => { const findings = validateFlowTemplatePaths( - flowWith({ title: '{some_var.field} {NOW()} {$User.Email}', body: 'x' }), + flowWith({ title: '{{ some_var.field }} {NOW()} {{ $User.Email }}', body: 'x' }), ); expect(findings).toHaveLength(0); }); @@ -119,7 +120,7 @@ describe('validateFlowTemplatePaths', () => { type: 'record_change', nodes: [ { id: 'start', type: 'start', config: { objectName: 'sys_user', triggerType: 'record-after-create' } }, - { id: 'n1', type: 'notify', notify: { title: '{record.anything.deep}', body: 'x' } }, + { id: 'n1', type: 'notify', notify: { title: '{{ record.anything.deep }}', body: 'x' } }, ], }, ], @@ -136,7 +137,7 @@ describe('validateFlowTemplatePaths', () => { type: 'screen', nodes: [ { id: 'start', type: 'start', config: {} }, - { id: 'n1', type: 'notify', notify: { title: '{record.full_naem}', body: 'x' } }, + { id: 'n1', type: 'notify', notify: { title: '{{ record.full_naem }}', body: 'x' } }, ], }, ], @@ -146,7 +147,7 @@ describe('validateFlowTemplatePaths', () => { it('dedupes a repeated bad reference to one finding per node', () => { const findings = validateFlowTemplatePaths( - flowWith({ title: '{record.full_naem}', body: 'again {record.full_naem}' }), + flowWith({ title: '{{ record.full_naem }}', body: 'again {{ record.full_naem }}' }), ); expect(findings).toHaveLength(1); }); @@ -160,7 +161,7 @@ describe('validateFlowTemplatePaths', () => { type: 'record_change', nodes: [ { id: 'start', type: 'start', start: { objectName: 'crm_lead', triggerType: 'record-after-create' } }, - { id: 'n1', type: 'notify', notify: { title: '{record.crm_account.name}', body: 'x' } }, + { id: 'n1', type: 'notify', notify: { title: '{{ record.crm_account.name }}', body: 'x' } }, ], }, ], @@ -200,7 +201,7 @@ describe('validateFlowTemplatePaths', () => { type: 'start', config: { objectName: 'crm_lead', triggerType: 'record-after-create', expand: ['crm_account'] }, }, - { id: 'n1', type: 'notify', notify: { title: 'From {record.crm_account.name}', body: 'x' } }, + { id: 'n1', type: 'notify', notify: { title: 'From {{ record.crm_account.name }}', body: 'x' } }, ], }, ], @@ -221,7 +222,7 @@ describe('validateFlowTemplatePaths', () => { type: 'start', config: { objectName: 'crm_lead', triggerType: 'record-after-create', expand: ['target_channels'] }, }, - { id: 'n1', type: 'notify', notify: { title: 'From {record.crm_account.name}', body: 'x' } }, + { id: 'n1', type: 'notify', notify: { title: 'From {{ record.crm_account.name }}', body: 'x' } }, ], }, ], @@ -234,11 +235,13 @@ describe('validateFlowTemplatePaths', () => { expect(validateFlowTemplatePaths({ objects: [LEAD_OBJECT] })).toHaveLength(0); }); - // ── severity follows position (#3810) ─────────────────────────────────── - // Outside a filter an unresolved token blanks a value (advisory); inside a - // filter-guarded CRUD node's filter it DELETES the condition, so the node - // refuses to run — the build must not ship it. - describe('filter-position severity (#3810)', () => { + // ── a CRUD filter operand is not scanned (#19939) ─────────────────────── + // Each operand of a get_record / update_record / delete_record `filter` is a + // value slot — a CEL envelope or a literal, rendering no template. A `{…}` + // token there, a `{{ }}` hole included, is the value-slot judge's refusal at + // every build door; this rule reports nothing there. (Until #19939 a token + // there erased its condition, and #3810's refusal made the finding an ERROR.) + describe('a CRUD filter operand (#19939)', () => { /** A record-change flow with one CRUD node carrying the given config. */ function crudFlow(type: string, config: AnyRec): AnyRec { return { @@ -257,102 +260,47 @@ describe('validateFlowTemplatePaths', () => { } it.each(['get_record', 'update_record', 'delete_record'])( - 'raises an unknown field inside a %s filter to error', + 'reports nothing at a %s filter operand — the value-slot judge refuses the hole there', (type) => { - const findings = validateFlowTemplatePaths( - crudFlow(type, { objectName: 'crm_lead', filter: { company: '{record.compnay}' } }), - ); - expect(findings).toHaveLength(1); - expect(findings[0].rule).toBe(FLOW_TEMPLATE_UNKNOWN_FIELD); - expect(findings[0].severity).toBe('error'); - expect(findings[0].message).toContain('refuses to run'); + const config = { objectName: 'crm_lead', filter: { company: '{{ record.compnay }}', owner: { $in: ['{{ record.crm_account.name }}'] } } }; + expect(validateFlowTemplatePaths(crudFlow(type, config))).toEqual([]); + expect(flowNodeValueTemplateRefusals(type, config).map((r) => r.path)).toEqual(['filter.company', 'filter.owner.$in[0]']); }, ); - it('raises a lookup traversal inside a filter to error', () => { - const findings = validateFlowTemplatePaths( - crudFlow('delete_record', { objectName: 'crm_lead', filter: { company: '{record.crm_account.name}' } }), - ); - expect(findings).toHaveLength(1); - expect(findings[0].rule).toBe(FLOW_TEMPLATE_LOOKUP_TRAVERSAL); - expect(findings[0].severity).toBe('error'); - }); - - it('keeps a bad reference OUTSIDE the filter advisory on the same node', () => { + it('still warns on a bad reference OUTSIDE the filter on the same node', () => { const findings = validateFlowTemplatePaths( crudFlow('update_record', { objectName: 'crm_lead', - filter: { company: '{record.company}' }, - fields: { last_name: '{record.compnay}' }, + filter: { company: '{{ record.compnay }}' }, + fields: { last_name: '{{ record.compnay }}' }, }), ); expect(findings).toHaveLength(1); expect(findings[0].severity).toBe('warning'); }); - it('reports a reference used in BOTH positions once, at error severity', () => { + it('scans a `filter` key on create_record, which has no filter operands', () => { const findings = validateFlowTemplatePaths( - crudFlow('update_record', { - objectName: 'crm_lead', - filter: { company: '{record.compnay}' }, - fields: { last_name: 'echo {record.compnay}' }, - }), - ); - expect(findings).toHaveLength(1); - expect(findings[0].severity).toBe('error'); - }); - - it('stays advisory for create_record, which has no filter to widen', () => { - const findings = validateFlowTemplatePaths( - crudFlow('create_record', { objectName: 'crm_lead', filter: { company: '{record.compnay}' } }), + crudFlow('create_record', { objectName: 'crm_lead', filter: { company: '{{ record.compnay }}' } }), ); expect(findings).toHaveLength(1); expect(findings[0].severity).toBe('warning'); }); - it('does NOT flag a valid filter reference', () => { - const findings = validateFlowTemplatePaths( - crudFlow('delete_record', { - objectName: 'crm_lead', - filter: { company: '{record.company}', owner: '{record.owner}' }, - }), - ); - expect(findings).toHaveLength(0); - }); - - it('respects declared expand for a filter traversal (#3475)', () => { - const findings = validateFlowTemplatePaths({ - objects: [LEAD_OBJECT], - flows: [ - { - name: 'expanded_filter', - type: 'record_change', - nodes: [ - { - id: 'start', - type: 'start', - config: { objectName: 'crm_lead', triggerType: 'record-after-create', expand: ['crm_account'] }, - }, - { - id: 'c1', - type: 'get_record', - config: { objectName: 'crm_lead', filter: { company: '{record.crm_account.name}' } }, - }, - ], - }, - ], - }); - expect(findings).toHaveLength(0); + it('reads no single-brace token — anywhere', () => { + expect(validateFlowTemplatePaths(flowWith({ title: 'New lead: {record.full_naem}', body: 'x' }))).toEqual([]); + expect(validateFlowTemplatePaths( + crudFlow('get_record', { objectName: 'crm_lead', filter: { company: '{record.compnay}' } }), + )).toEqual([]); }); }); // ── nested regions (#4380) ───────────────────────────────────────────── // - // This rule was not merely blind to nested nodes — it was WORSE than blind. - // The recursive string-leaf scan already saw a nested node's tokens through - // its container's `config`, but the `filter` split only looked at the top - // level of the node it was handed, so a nested filter token lost its position - // and the gating #3810 finding silently degraded to a warning reported - // against the wrapping `try_catch`. + // The recursive string-leaf scan already saw a nested node's holes through + // its container's `config`, but judged them against the wrapping container. + // Walking to the real node restores the location (and, since #19939, the + // CRUD filter exemption, which a nested CRUD node needs too). describe('nested regions', () => { const nestedFilterFlow = (container: Record) => ({ objects: [LEAD_OBJECT], @@ -367,31 +315,40 @@ describe('validateFlowTemplatePaths', () => { }, ], }); - const badFilterNode = { + const badNoteNode = { + id: 'note', + type: 'notify', + label: 'Note', + config: { title: 'Budget: {{ record.budget }}' }, + }; + const filterNode = { id: 'fetch', type: 'get_record', label: 'Fetch', - config: { objectName: 'crm_lead', filter: { company: '{record.budget}' } }, + config: { objectName: 'crm_lead', filter: { company: '{{ record.budget }}' } }, }; - it('keeps the gating filter-position severity inside a region', () => { + it('reports a nested hole on the node that carries it', () => { const findings = validateFlowTemplatePaths( - nestedFilterFlow({ try: { nodes: [badFilterNode], edges: [] } }), + nestedFilterFlow({ try: { nodes: [badNoteNode], edges: [] } }), ); expect(findings).toHaveLength(1); - // The whole point: `error`, not the `warning` it used to degrade to. - expect(findings[0].severity).toBe('error'); + expect(findings[0].severity).toBe('warning'); expect(findings[0].path).toBe('flows[0].nodes[1].config.try.nodes[0]'); - expect(findings[0].where).toBe('flow "guarded" try_catch "Guard" › try node "get_record"'); + expect(findings[0].where).toBe('flow "guarded" try_catch "Guard" › try node "notify"'); }); it('reports a nested token once, not also against the container', () => { const findings = validateFlowTemplatePaths( - nestedFilterFlow({ catch: { nodes: [badFilterNode], edges: [] } }), + nestedFilterFlow({ catch: { nodes: [badNoteNode], edges: [] } }), ); expect(findings).toHaveLength(1); }); + it('exempts a nested CRUD node\'s filter operands too, not only a top-level one', () => { + expect(validateFlowTemplatePaths(nestedFilterFlow({ try: { nodes: [filterNode], edges: [] } }))).toEqual([]); + }); + it('still checks the container node\'s own config', () => { const findings = validateFlowTemplatePaths({ objects: [LEAD_OBJECT], @@ -408,7 +365,7 @@ describe('validateFlowTemplatePaths', () => { // The loop's OWN collection token is a non-filter position on // the container itself — warning, and not swallowed by the // region-stripping that prevents double reporting. - config: { collection: '{record.budget}', body: { nodes: [], edges: [] } }, + config: { collection: '{{ record.budget }}', body: { nodes: [], edges: [] } }, }, ], }, @@ -422,9 +379,9 @@ describe('validateFlowTemplatePaths', () => { // ── a template outside a node filter ─────────────────────────────────── // - // Both rule ids declare a per-position severity (`inFilter ? 'error' : - // 'warning'`) and a second message for the non-filter half, and that half was - // unreachable on the shape a real hand-off flow has: `body` is a region slot + // Both rule ids used to declare a per-position severity (`error` in a CRUD + // filter, `warning` elsewhere) and a second message for the non-filter half, + // and that half was unreachable on the shape a real hand-off flow has: `body` is a region slot // on `loop`, so the region-stripped view a recursive scan must read deleted // `config.body` from EVERY node — including the `http` node whose request // payload it is. The `http` executor interpolated its raw config wholesale, @@ -439,12 +396,12 @@ describe('validateFlowTemplatePaths', () => { // keeps an http node's own `config.body` is `flow-walk.test.ts`'s. // // Four pins, one fixture, matching the four injections the report measured. - // The two filter pins are NEGATIVE CONTROLS: they pass before this fix and - // must keep passing, or a later refactor could break the gating half while - // the warning half stays green. `warning` does not move the exit code, so + // [#19939] The two filter pins are re-judged: a filter operand is a value + // slot, and this rule no longer scans one, so whatever the filter's hole + // names it reports nothing there. `warning` does not move the exit code, so // every assertion below reads the findings array, never a pass/fail verdict. describe('outside a node filter', () => { - const CLEAN_FILTER = '{record.crm_account}'; + const CLEAN_FILTER = '{{ record.crm_account }}'; const CLEAN_PAYLOAD = '{{ record.company }}'; /** A billing hand-off: a filter-guarded read, then an http POST carrying the amount in a header. */ @@ -477,21 +434,11 @@ describe('validateFlowTemplatePaths', () => { ], }); - it('gates an unknown field in a filter (negative control)', () => { - const findings = validateFlowTemplatePaths(handoff('{record.crm_account_nope}', CLEAN_PAYLOAD)); - expect(findings).toHaveLength(1); - expect(findings[0].rule).toBe(FLOW_TEMPLATE_UNKNOWN_FIELD); - expect(findings[0].severity).toBe('error'); - expect(findings[0].where).toBe('flow "billing_handoff" node "get_record"'); - }); - - it('gates a lookup traversal in a filter (negative control)', () => { - const findings = validateFlowTemplatePaths(handoff('{record.crm_account.owner_id}', CLEAN_PAYLOAD)); - expect(findings).toHaveLength(1); - expect(findings[0].rule).toBe(FLOW_TEMPLATE_LOOKUP_TRAVERSAL); - expect(findings[0].severity).toBe('error'); - expect(findings[0].where).toBe('flow "billing_handoff" node "get_record"'); - }); + it.each(['{{ record.crm_account_nope }}', '{{ record.crm_account.owner_id }}'])( + 'reports nothing at the filter, whose operand %s is a value slot', (filterId) => { + expect(validateFlowTemplatePaths(handoff(filterId, CLEAN_PAYLOAD))).toEqual([]); + }, + ); it('warns on an unknown field in a payload outside any filter', () => { const findings = validateFlowTemplatePaths(handoff(CLEAN_FILTER, '{{ record.amount_nope }}')); @@ -516,7 +463,7 @@ describe('validateFlowTemplatePaths', () => { expect(validateFlowTemplatePaths(handoff(CLEAN_FILTER, CLEAN_PAYLOAD))).toEqual([]); }); - it('resolves one leaf used in both positions of a node at the gating severity', () => { + it('reports a leaf used in both positions once, from the position it scans', () => { const findings = validateFlowTemplatePaths({ objects: [LEAD_OBJECT], flows: [ @@ -531,8 +478,8 @@ describe('validateFlowTemplatePaths', () => { label: 'Touch', config: { objectName: 'crm_lead', - filter: { company: '{record.full_naem}' }, - fields: { company: 'echo {record.full_naem}' }, + filter: { company: '{{ record.full_naem }}' }, + fields: { company: 'echo {{ record.full_naem }}' }, }, }, ], @@ -540,7 +487,7 @@ describe('validateFlowTemplatePaths', () => { ], }); expect(findings).toHaveLength(1); - expect(findings[0].severity).toBe('error'); + expect(findings[0].severity).toBe('warning'); }); it('reports a payload nested in a loop body once, on the node that carries it', () => { @@ -611,41 +558,37 @@ describe('validateFlowTemplatePaths — unprovisioned injected anchors (#8340)', const only = (findings: ReturnType) => findings.filter((f) => f.rule === FLOW_TEMPLATE_FIELD_UNPROVISIONED); - it('warns on a filter token over an unprovisioned anchor — the existence rule stays silent', () => { + it('warns on a hole over an unprovisioned anchor, naming the blank-string consequence — the existence rule stays silent', () => { const findings = validateFlowTemplatePaths( - extFlow('get_record', { objectName: 'ext_customer', filter: { email: '{record.owner_id}' } }), + extFlow('notify', { title: 'Owned by {{ record.owner_id }}' }), ); expect(findings.filter((f) => f.rule === FLOW_TEMPLATE_UNKNOWN_FIELD)).toHaveLength(0); const warned = only(findings); expect(warned).toHaveLength(1); - // WARNING even in the filter position, where a typo would be an ERROR: - // the provenance question has no closed oracle here (#8116). + // A warning: the provenance question has no closed oracle here (#8116). expect(warned[0].severity).toBe('warning'); expect(warned[0].message).toContain('owner_id'); expect(warned[0].message).toContain('external object (ADR-0015)'); - expect(warned[0].message).toContain('refuses to run'); + expect(warned[0].message).toContain('empty string on every run'); expect(warned[0].hint).toContain('columnMap'); }); - it('warns outside a filter too, naming the blank-string consequence', () => { - const findings = validateFlowTemplatePaths( - extFlow('notify', { title: 'Owned by {record.owner_id}' }), - ); - const warned = only(findings); - expect(warned).toHaveLength(1); - expect(warned[0].message).toContain('empty string on every run'); + it('reports nothing at a CRUD filter operand over the anchor — a value slot (#19939)', () => { + expect(only(validateFlowTemplatePaths( + extFlow('get_record', { objectName: 'ext_customer', filter: { email: '{{ record.owner_id }}' } }), + ))).toEqual([]); }); it('is silent on the local twin — platform storage is real (mutation: drop `external`)', () => { const findings = validateFlowTemplatePaths( - extFlow('notify', { title: '{record.owner_id}' }, EXT_OBJECT({ external: undefined })), + extFlow('notify', { title: '{{ record.owner_id }}' }, EXT_OBJECT({ external: undefined })), ); expect(findings).toEqual([]); }); it('is silent when the author DECLARES the column (#7859)', () => { const findings = validateFlowTemplatePaths( - extFlow('notify', { title: '{record.owner_id}' }, EXT_OBJECT({ + extFlow('notify', { title: '{{ record.owner_id }}' }, EXT_OBJECT({ fields: { email: { name: 'email', type: 'text' }, owner_id: { name: 'owner_id', type: 'text' } }, })), ); @@ -653,15 +596,14 @@ describe('validateFlowTemplatePaths — unprovisioned injected anchors (#8340)', }); it('is silent on a declared field of the same external object', () => { - expect(validateFlowTemplatePaths(extFlow('notify', { title: '{record.email}' }))).toEqual([]); + expect(validateFlowTemplatePaths(extFlow('notify', { title: '{{ record.email }}' }))).toEqual([]); }); it('reports one finding per node for a token repeated in two positions', () => { const findings = validateFlowTemplatePaths( - extFlow('update_record', { - objectName: 'ext_customer', - filter: { email: '{record.owner_id}' }, - fields: { email: 'echo {record.owner_id}' }, + extFlow('notify', { + title: '{{ record.owner_id }}', + message: 'echo {{ record.owner_id }}', }), ); expect(only(findings)).toHaveLength(1); @@ -676,9 +618,9 @@ describe('validateFlowTemplatePaths — unprovisioned injected anchors (#8340)', // // | token root | kind | seen before #17305 | // |---------------------------------|--------------------|--------------------| -// | `{caseRecord.owner_id.manager}` | `get_record` output| no | -// | `{currentCase.owner_id.manager}`| `loop` iterator | no | -// | `{record.owner_id.manager}` | trigger record | yes | +// | `{{ caseRecord.owner_id.manager }}` | `get_record` output| no | +// | `{{ currentCase.owner_id.manager }}`| `loop` iterator | no | +// | `{{ record.owner_id.manager }}` | trigger record | yes | // // Both invisible shapes sit on a `schedule` flow in the report, which is the // second dimension: the rule skipped a non-record-triggered flow whole. Every @@ -727,17 +669,17 @@ describe('validateFlowTemplatePaths — variable roots (#17305)', () => { const findings = validateFlowTemplatePaths( scheduleFlow([ FETCH_ONE, - { id: 'escalate', type: 'notify', config: { body: 'Escalating to {caseRecord.owner_id.manager}' } }, + { id: 'escalate', type: 'notify', config: { body: 'Escalating to {{ caseRecord.owner_id.manager }}' } }, ]), ); expect(findings).toHaveLength(1); expect(findings[0].rule).toBe(FLOW_TEMPLATE_LOOKUP_TRAVERSAL); expect(findings[0].severity).toBe('warning'); expect(findings[0].path).toBe('flows[0].nodes[2]'); - expect(findings[0].message).toContain('{caseRecord.owner_id.manager}'); + expect(findings[0].message).toContain('{{ caseRecord.owner_id.manager }}'); // The object is named from the BINDING node, not from a trigger there is none of. expect(findings[0].message).toContain("'owner_id'"); - expect(findings[0].hint).toContain('{caseRecord.owner_id}'); + expect(findings[0].hint).toContain('{{ caseRecord.owner_id }}'); // config.expand is the START node's opt-in and must not be prescribed here. expect(findings[0].hint).toContain("START node's opt-in"); }); @@ -746,7 +688,7 @@ describe('validateFlowTemplatePaths — variable roots (#17305)', () => { const findings = validateFlowTemplatePaths( scheduleFlow([ FETCH_ONE, - { id: 'escalate', type: 'notify', config: { body: 'Case {caseRecord.subjcet}' } }, + { id: 'escalate', type: 'notify', config: { body: 'Case {{ caseRecord.subjcet }}' } }, ]), ); expect(findings).toHaveLength(1); @@ -755,20 +697,18 @@ describe('validateFlowTemplatePaths — variable roots (#17305)', () => { expect(findings[0].message).toContain("'subjcet' is not a field on object 'crm_case'"); }); - it('gates a variable-root hop in a filter-guarded position (#3810 severity split)', () => { + it('reports nothing for a variable-root hop at a CRUD filter operand — a value slot (#19939)', () => { const findings = validateFlowTemplatePaths( scheduleFlow([ FETCH_ONE, { id: 'close', type: 'delete_record', - config: { objectName: 'crm_case', filter: { owner_id: '{caseRecord.owner_id.manager}' } }, + config: { objectName: 'crm_case', filter: { owner_id: '{{ caseRecord.owner_id.manager }}' } }, }, ]), ); - expect(findings).toHaveLength(1); - expect(findings[0].severity).toBe('error'); - expect(findings[0].message).toContain('refuses to run at execution time'); + expect(findings).toEqual([]); }); it('resolves a get_record output on a RECORD-TRIGGERED flow too (the root dimension alone)', () => { @@ -781,13 +721,13 @@ describe('validateFlowTemplatePaths — variable roots (#17305)', () => { nodes: [ { id: 'start', type: 'start', config: { objectName: 'crm_case', triggerType: 'record-after-create' } }, { id: 'fetch_one', type: 'get_record', config: { objectName: 'crm_case', outputVariable: 'peerCase' } }, - { id: 'note', type: 'notify', config: { body: '{peerCase.owner_id.manager}' } }, + { id: 'note', type: 'notify', config: { body: '{{ peerCase.owner_id.manager }}' } }, ], }, ], }); expect(findings).toHaveLength(1); - expect(findings[0].message).toContain('{peerCase.owner_id.manager}'); + expect(findings[0].message).toContain('{{ peerCase.owner_id.manager }}'); }); // ── shape 2: a `loop` iterator root ─────────────────────────────────────── @@ -804,7 +744,7 @@ describe('validateFlowTemplatePaths — variable roots (#17305)', () => { collection: { dialect: 'cel', source: 'staleCases' }, iteratorVariable: 'currentCase', body: { - nodes: [{ id: 'ping', type: 'notify', config: { body: 'Ping {currentCase.owner_id.manager}' } }], + nodes: [{ id: 'ping', type: 'notify', config: { body: 'Ping {{ currentCase.owner_id.manager }}' } }], edges: [], }, }, @@ -815,16 +755,17 @@ describe('validateFlowTemplatePaths — variable roots (#17305)', () => { expect(findings[0].rule).toBe(FLOW_TEMPLATE_LOOKUP_TRAVERSAL); expect(findings[0].path).toBe('flows[0].nodes[2].config.body.nodes[0]'); expect(findings[0].where).toBe('flow "case_sla_monitor" loop "Each stale case" › body node "notify"'); - expect(findings[0].message).toContain('{currentCase.owner_id.manager}'); + expect(findings[0].message).toContain('{{ currentCase.owner_id.manager }}'); }); - // [#19939] RE-JUDGED IN PLACE: this pinned the bare name as "the spelling - // loop-node.ts falls back to" — the structured loop never read one (the - // interpolator handed the name back and the node failed). Every door refuses - // both pre-#19939 spellings now; the rule still reads them, so a flow - // carrying one keeps its template-path findings beside that refusal. + // [#19939] RE-JUDGED IN PLACE, twice: this pinned the bare name as "the + // spelling loop-node.ts falls back to" — the structured loop never read one + // (the interpolator handed the name back and the node failed) — and then as a + // pre-#19939 spelling the rule kept reading. Every door refuses both, and + // since S7 deleted the single-brace dialect's last reader the rule reads + // neither: such a loop binds no root, so the body is silent. it.each([['a lone token', '{staleCases}'], ['a bare name', 'staleCases']])( - 'still resolves a pre-#19939 collection spelling, %s', (_label, collection) => { + 'resolves no root from a pre-#19939 collection spelling, %s', (_label, collection) => { const findings = validateFlowTemplatePaths( scheduleFlow([ FETCH_MANY, @@ -834,13 +775,12 @@ describe('validateFlowTemplatePaths — variable roots (#17305)', () => { config: { collection, iteratorVariable: 'currentCase', - body: { nodes: [{ id: 'ping', type: 'notify', config: { body: '{currentCase.subjcet}' } }], edges: [] }, + body: { nodes: [{ id: 'ping', type: 'notify', config: { body: '{{ currentCase.subjcet }}' } }], edges: [] }, }, }, ]), ); - expect(findings).toHaveLength(1); - expect(findings[0].rule).toBe(FLOW_TEMPLATE_UNKNOWN_FIELD); + expect(findings).toEqual([]); }, ); @@ -854,7 +794,7 @@ describe('validateFlowTemplatePaths — variable roots (#17305)', () => { config: { collection: { dialect: 'cel', source: 'staleCases.filter(c, c.priority == 1)' }, iteratorVariable: 'currentCase', - body: { nodes: [{ id: 'ping', type: 'notify', config: { body: '{currentCase.subjcet}' } }], edges: [] }, + body: { nodes: [{ id: 'ping', type: 'notify', config: { body: '{{ currentCase.subjcet }}' } }], edges: [] }, }, }, ]), @@ -878,23 +818,22 @@ describe('validateFlowTemplatePaths — variable roots (#17305)', () => { }, ], }); - const warned = validateFlowTemplatePaths(triggered({ body: '{record.owner_id.manager}' })); + const warned = validateFlowTemplatePaths(triggered({ body: '{{ record.owner_id.manager }}' })); expect(warned).toHaveLength(1); expect(warned[0].rule).toBe(FLOW_TEMPLATE_LOOKUP_TRAVERSAL); expect(warned[0].severity).toBe('warning'); expect(warned[0].hint).toContain("config.expand and the engine re-reads it as the run's identity"); - const gated = validateFlowTemplatePaths( - triggered({ objectName: 'crm_case', filter: { status: '{record.owner_id.manager}' } }, 'update_record'), - ); - expect(gated).toHaveLength(1); - expect(gated[0].severity).toBe('error'); + // [#19939] A CRUD filter operand is a value slot: no longer scanned, at any root. + expect(validateFlowTemplatePaths( + triggered({ objectName: 'crm_case', filter: { status: '{{ record.owner_id.manager }}' } }, 'update_record'), + )).toEqual([]); }); it('keeps {record.…} unjudged on a flow with no record trigger', () => { expect( validateFlowTemplatePaths( - scheduleFlow([{ id: 'note', type: 'notify', config: { body: '{record.subjcet}' } }]), + scheduleFlow([{ id: 'note', type: 'notify', config: { body: '{{ record.subjcet }}' } }]), ), ).toEqual([]); }); @@ -904,7 +843,7 @@ describe('validateFlowTemplatePaths — variable roots (#17305)', () => { it('is silent on a valid field read off a variable root (no false positive)', () => { expect( validateFlowTemplatePaths( - scheduleFlow([FETCH_ONE, { id: 'note', type: 'notify', config: { body: '{caseRecord.subject}' } }]), + scheduleFlow([FETCH_ONE, { id: 'note', type: 'notify', config: { body: '{{ caseRecord.subject }}' } }]), ), ).toEqual([]); }); @@ -917,7 +856,7 @@ describe('validateFlowTemplatePaths — variable roots (#17305)', () => { it('still resolves the root when the flow DECLARES the variable it binds', () => { const findings = validateFlowTemplatePaths( scheduleFlow( - [FETCH_ONE, { id: 'note', type: 'notify', config: { body: '{caseRecord.subjcet}' } }], + [FETCH_ONE, { id: 'note', type: 'notify', config: { body: '{{ caseRecord.subjcet }}' } }], { variables: [{ name: 'caseRecord', type: 'object', isInput: false, isOutput: false }] }, ), ); @@ -979,7 +918,7 @@ describe('validateFlowTemplatePaths — variable roots (#17305)', () => { [ FETCH_MANY, { id: 'loop_cases', type: 'loop', config: { collection: { dialect: 'cel', source: 'staleCases' }, iteratorVariable: 'currentCase' } }, - { id: 'note', type: 'notify', config: { title: '{currentCase.subject}', body: '{currentCase.subjcet}' } }, + { id: 'note', type: 'notify', config: { title: '{{ currentCase.subject }}', body: '{{ currentCase.subjcet }}' } }, ], { variables: [{ name: 'staleCases', type: 'record_collection', isInput: false, isOutput: false }] }, ), @@ -987,7 +926,7 @@ describe('validateFlowTemplatePaths — variable roots (#17305)', () => { // The loop body lives in the EDGE graph, not a region slot — the root map // is flow-scoped, so a token on a top-level node still resolves. expect(findings).toHaveLength(1); - expect(findings[0].message).toContain('{currentCase.subjcet}'); + expect(findings[0].message).toContain('{{ currentCase.subjcet }}'); }); it('is silent when an assignment node also writes the name', () => { @@ -996,7 +935,7 @@ describe('validateFlowTemplatePaths — variable roots (#17305)', () => { scheduleFlow([ FETCH_ONE, { id: 'seed', type: 'assignment', config: { assignments: { caseRecord: 'x' } } }, - { id: 'note', type: 'notify', config: { body: '{caseRecord.subjcet}' } }, + { id: 'note', type: 'notify', config: { body: '{{ caseRecord.subjcet }}' } }, ]), ), ).toEqual([]); @@ -1014,7 +953,7 @@ describe('validateFlowTemplatePaths — variable roots (#17305)', () => { { id: 'start', type: 'start', config: { triggerType: 'schedule' } }, { id: 'a', type: 'get_record', config: { objectName: 'crm_case', outputVariable: 'row' } }, { id: 'b', type: 'get_record', config: { objectName: 'crm_lead2', outputVariable: 'row' } }, - { id: 'note', type: 'notify', config: { body: '{row.subjcet}' } }, + { id: 'note', type: 'notify', config: { body: '{{ row.subjcet }}' } }, ], }, ], @@ -1027,7 +966,7 @@ describe('validateFlowTemplatePaths — variable roots (#17305)', () => { validateFlowTemplatePaths( scheduleFlow([ { id: 'caseRecord', type: 'get_record', config: { objectName: 'crm_case', outputVariable: 'caseRecord' } }, - { id: 'note', type: 'notify', config: { body: '{caseRecord.subjcet}' } }, + { id: 'note', type: 'notify', config: { body: '{{ caseRecord.subjcet }}' } }, ]), ), ).toEqual([]); @@ -1044,7 +983,7 @@ describe('validateFlowTemplatePaths — variable roots (#17305)', () => { nodes: [ { id: 'start', type: 'start', config: { objectName: 'crm_case', triggerType: 'record-after-create' } }, { id: 'fetch', type: 'get_record', config: { objectName: 'crm_case', outputVariable: 'status' } }, - { id: 'note', type: 'notify', config: { body: '{status.subjcet}' } }, + { id: 'note', type: 'notify', config: { body: '{{ status.subjcet }}' } }, ], }, ], @@ -1057,7 +996,7 @@ describe('validateFlowTemplatePaths — variable roots (#17305)', () => { validateFlowTemplatePaths( scheduleFlow([ { id: 'ask', type: 'screen', config: { objectName: 'crm_case', outputVariable: 'answer' } }, - { id: 'note', type: 'notify', config: { body: '{answer.subjcet}' } }, + { id: 'note', type: 'notify', config: { body: '{{ answer.subjcet }}' } }, ]), ), ).toEqual([]); @@ -1066,7 +1005,7 @@ describe('validateFlowTemplatePaths — variable roots (#17305)', () => { it('is silent on a multi-record output read as if it were one record', () => { expect( validateFlowTemplatePaths( - scheduleFlow([FETCH_MANY, { id: 'note', type: 'notify', config: { body: '{staleCases.subjcet}' } }]), + scheduleFlow([FETCH_MANY, { id: 'note', type: 'notify', config: { body: '{{ staleCases.subjcet }}' } }]), ), ).toEqual([]); }); @@ -1081,7 +1020,7 @@ describe('validateFlowTemplatePaths — variable roots (#17305)', () => { config: { collection: { dialect: 'cel', source: 'somethingElse' }, iteratorVariable: 'currentCase', - body: { nodes: [{ id: 'ping', type: 'notify', config: { body: '{currentCase.subjcet}' } }], edges: [] }, + body: { nodes: [{ id: 'ping', type: 'notify', config: { body: '{{ currentCase.subjcet }}' } }], edges: [] }, }, }, ]), @@ -1099,7 +1038,7 @@ describe('validateFlowTemplatePaths — variable roots (#17305)', () => { nodes: [ { id: 'start', type: 'start', config: { triggerType: 'schedule' } }, { id: 'fetch', type: 'get_record', config: { objectName: 'ext_customer', outputVariable: 'customer' } }, - { id: 'note', type: 'notify', config: { body: '{customer.owner_id}' } }, + { id: 'note', type: 'notify', config: { body: '{{ customer.owner_id }}' } }, ], }, ], diff --git a/packages/lint/src/validate-flow-template-paths.ts b/packages/lint/src/validate-flow-template-paths.ts index 7e3855d7bec..ec7dbaf00b6 100644 --- a/packages/lint/src/validate-flow-template-paths.ts +++ b/packages/lint/src/validate-flow-template-paths.ts @@ -1,26 +1,27 @@ // Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license. // -// Build-time guardrail for `{record.}` template references in a +// Build-time guardrail for `{{ record. }}` template references in a // record-change flow's node config (#3426). // -// A `notify` / `update_record` / `http` / ... node resolves `{record.}` -// references against the triggering record — a `{{ }}` hole in a text slot, a -// single-brace token where that dialect still reads. Two authoring -// mistakes render a SILENT empty string at runtime, with no design-time -// signal — exactly the failure #3426 reported: +// A `notify` / `http` / `screen` / ... node renders `{{ record. }}` +// holes against the triggering record — the `{{ }}` holes of its text slots +// (ADR-0032 D3; the single-brace `{…}` dialect is retired from every flow +// position and its interpolator deleted, #19939). Two authoring mistakes +// render a SILENT empty string at runtime, with no design-time signal — +// exactly the failure #3426 reported: // -// 1. `{record.}` — the path head is neither a declared field nor a -// system column. Almost always a typo (`{record.full_naem}`). The template +// 1. `{{ record. }}` — the path head is neither a declared field nor a +// system column. Almost always a typo (`{{ record.full_naem }}`). The template // engine resolves it to `undefined` -> '' with no warning. // -// 2. `{record..}` — a cross-object hop through a lookup / +// 2. `{{ record.. }}` — a cross-object hop through a lookup / // master_detail / user / tree relation. The seeded flow record carries the // relation as a SCALAR foreign-key id, not an expanded object (a default // data-API read does not expand relations either — see #3426's hydration // note and #1872). So `record.account.name` walks `.name` on a string id // and yields '' silently. Not resolved today; tracked on #3426. // -// 3. `{record.}` on an ADR-0015 `external` trigger object +// 3. `{{ record. }}` on an ADR-0015 `external` trigger object // (#8340) — the head RESOLVES (it is a registry-injected system column, so // case 1 rightly stays silent), but the remote database owns the table and // the platform provisions no storage behind the anchor. The value is empty @@ -35,25 +36,17 @@ // A pure `(stack) => Finding[]` rule (ADR-0019), run from `os validate` and // reusable by AI authoring. // -// SEVERITY FOLLOWS THE RUNTIME CONSEQUENCE, which differs by POSITION: +// SEVERITY: WARNING. An unresolved hole renders a blank: the output is wrong +// but the run completes, and the head object may legitimately come from +// another installed package (skipped — see below). // -// - Everywhere else (a message body, an http url, a write payload) an -// unresolved token renders a blank. The output is wrong but the run -// completes, and the head object may legitimately come from another -// installed package (skipped — see below). Advisory: WARNING. -// -// - Inside a filter-guarded CRUD node's `filter`, an unresolved token used to -// DELETE the condition from the query, and a removed condition matches MORE -// rows — `delete_record` with its only condition gone matched every row. -// Since framework#3810 those nodes REFUSE TO EXECUTE. So a finding here is -// not "the output will be blank", it is "this node cannot run": the build -// is shipping a flow whose runtime is already decided. Gating: ERROR. -// -// The split is the same shift-left `validateReadonlyFlowWrites` makes — a -// certain runtime failure gates the build, a state-dependent one advises — and -// it keeps this rule honest about what it found. Catching the typo at build -// time beats a failed run at 3am; catching it and calling it advisory, when the -// runtime has already committed to refusing, understates it. +// A CRUD node's `filter` is NOT scanned (#19939). Each of its operands is a +// value slot — a CEL value envelope evaluated before the query, or a literal +// handed to it as written — which renders no template in either dialect, and +// a `{…}` token there (a `{{ }}` hole included) is refused at every build door +// by the value-slot judge. The ERROR severity this rule gave a filter token — +// the token used to erase its condition, and framework#3810 refuses such a +// node — went with the dialect that read one there. // // Deliberately conservative to keep false positives near zero: // - A token is checked only when its ROOT resolves to an object THIS stack @@ -159,14 +152,11 @@ const RELATION_TYPES: ReadonlySet = new Set([ 'tree', ]); -// The CRUD nodes whose `filter` the runtime guards (framework#3810): each calls -// `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([ +// The CRUD nodes whose `filter` operands are value slots (#19939): a CEL value +// envelope or a literal, which renders no template, so this rule does not scan +// the `filter` of these nodes. `create_record` is deliberately absent — it has +// no filter, and a `filter` key on it is no operand. +const FILTER_VALUE_SLOT_NODE_TYPES: ReadonlySet = new Set([ 'get_record', 'update_record', 'delete_record', @@ -183,35 +173,29 @@ function fieldTypesOf(obj: AnyRec): Map { return types; } -/** One dotted `{root.seg…}` reference found in a template string. */ +/** One dotted `{{ root.seg… }}` reference found in a template string. */ interface TemplateRef { - /** The token's first segment — the name the run resolves in its variable map. */ + /** The hole's first segment — the name the run resolves in its variable map. */ root: string; - /** The segments AFTER the root, e.g. `{record.account.name}` -> `['account', 'name']`. */ + /** The segments AFTER the root, e.g. `{{ record.account.name }}` -> `['account', 'name']`. */ rest: string[]; } /** - * Extract the dotted `{root.}` references from a template string, in - * either of the two dialects a flow config string carries: + * Extract the dotted `{{ root. }}` references from a template string — + * the `{{ ... }}` holes of the formula template engine, which the text slots + * render (#22110: a notify `title` / `message` and, since #19939, its + * `actionUrl` and an http `url` and `headers` values, a screen `title` / + * `description`, an `end` `message`): a path with an optional + * `| formatter[:arg]`, whose segments may be bracket-indexed + * (`{{ rows[0].subject }}`). The path before the pipe, with each `[i]` read as + * `.i` the way the engine resolves it, is judged exactly like a bare one, so + * neither a formatter nor an index hides a misspelt field. * - * - a single-brace `{...}` token — the grammar of the flow interpolator that - * still renders every value-like position (service-automation - * `builtin/template.ts`, `interpolate`): a token whose body is a plain - * dotted path; - * - a `{{ ... }}` hole — the formula template engine's grammar, which the - * text slots render since #22110 (a notify `title` / `message` and, since - * #19939, its `actionUrl` and an http `url` and `headers` values, a screen - * `title` / `description`, an `end` `message`): a path with an optional - * `| formatter[:arg]`, whose segments may be bracket-indexed - * (`{{ rows[0].subject }}`). The path before the pipe, with each `[i]` - * read as `.i` the way the engine resolves it, is judged exactly like a - * bare one, so neither a formatter nor an index hides a misspelt field — - * before #22110 a text slot had neither syntax, and every path in one was - * judged. - * - * Arithmetic / function tokens (`{NOW()}`, `{a + b}`) are ignored, and so is a - * single-segment token — there is no `.` hop in it to judge. + * A single-brace `{...}` token is NOT read (#19939): no flow position renders + * that dialect any more — its interpolator is deleted — and every door refuses + * it with its replacement. A hole holding anything but a path, and a + * single-segment hole, are ignored: there is no `.` hop in it to judge. * * The ROOT is returned rather than filtered here (#17305): which roots are * resolvable is the caller's per-flow question, and it is no longer the single @@ -219,21 +203,17 @@ interface TemplateRef { */ function templateRefsIn(text: string): TemplateRef[] { const refs: TemplateRef[] = []; - const tokenRe = /\{([^{}]+)\}/g; + const holeRe = /\{\{([^{}]+)\}\}/g; let m: RegExpExecArray | null; - while ((m = tokenRe.exec(text)) !== null) { + while ((m = holeRe.exec(text)) !== null) { let body = m[1].trim(); - // Inside a `{{ }}` hole — a brace on both sides — the path is the text - // before an optional `| formatter`. (In a single-brace token a `|` is the - // interpolator's arithmetic, so it stays unjudged there, as before.) - const inHole = text[m.index - 1] === '{' && text[m.index + m[0].length] === '}'; - if (inHole && body.includes('|')) body = body.slice(0, body.indexOf('|')).trim(); + // The path is the text before an optional `| formatter`… + if (body.includes('|')) body = body.slice(0, body.indexOf('|')).trim(); // …and an index segment is the engine's `[i]` → `.i` (formula's // `template-engine.ts` `resolvePath` normalises the same way). - if (inHole) body = body.replace(/\[(\w+)\]/g, '.$1'); - // Pure dotted path only (same shape the interpolator's fast path accepts): - // identifier head, then identifier-or-numeric segments. Anything with - // operators / spaces / quotes is an arithmetic token — not a bare field ref. + body = body.replace(/\[(\w+)\]/g, '.$1'); + // Pure dotted path only: identifier head, then identifier-or-numeric + // segments. Anything with operators / spaces / quotes is no field ref. if (!/^[A-Za-z_$][\w$]*(?:\.(?:[A-Za-z_$][\w$]*|\d+))*$/.test(body)) continue; const segments = body.split('.'); const rest = segments.slice(1); @@ -257,10 +237,9 @@ function stringLeaves(value: unknown, out: string[]): void { } } -// The typed config blocks + freeform `config` a node interpolates at runtime. -// We scan every string leaf under these (the runtime `interpolate()` walks the -// whole config recursively), NOT `id` / `type` / `label` / `position`, which are -// never templated. +// The typed config blocks + freeform `config` a node's templates live in. We +// scan every string leaf under these, NOT `id` / `type` / `label` / +// `position`, which are never templated. const NODE_CONFIG_KEYS = [ 'config', 'notify', @@ -277,47 +256,24 @@ const NODE_CONFIG_KEYS = [ 'start', ]; -/** A templated string leaf plus the one thing severity depends on: where it sits. */ -interface TemplateLeaf { - text: string; - /** Inside a filter-guarded CRUD node's `filter` — an unresolved token there is refused at runtime. */ - inFilter: boolean; -} - /** - * Collect a node's templated string leaves, tagging those that sit under a - * `filter` key when the node type is one the runtime guards. - * - * `guarded` leaves are returned FIRST so the per-node dedupe below resolves a - * reference that appears in both positions at its higher severity: one typo - * used in a filter and echoed in a message is an error, not a warning. + * Collect a node's templated string leaves — every one but those under a + * `filter` key when the node type's filter operands are value slots + * ({@link FILTER_VALUE_SLOT_NODE_TYPES}), which render no template. */ -function collectNodeLeaves(node: AnyRec, guarded: boolean): TemplateLeaf[] { - const filterLeaves: TemplateLeaf[] = []; - const otherLeaves: TemplateLeaf[] = []; - +function collectNodeLeaves(node: AnyRec, skipFilter: boolean): string[] { + const leaves: string[] = []; for (const key of NODE_CONFIG_KEYS) { if (!(key in node)) continue; const block = node[key]; - const splitFilter = guarded && !!block && typeof block === 'object' && !Array.isArray(block); - - if (splitFilter) { - const { filter, ...rest } = block as AnyRec; - const inFilter: string[] = []; - stringLeaves(filter, inFilter); - for (const text of inFilter) filterLeaves.push({ text, inFilter: true }); - const outside: string[] = []; - stringLeaves(rest, outside); - for (const text of outside) otherLeaves.push({ text, inFilter: false }); + if (skipFilter && !!block && typeof block === 'object' && !Array.isArray(block)) { + const { filter: _operands, ...rest } = block as AnyRec; + stringLeaves(rest, leaves); continue; } - - const plain: string[] = []; - stringLeaves(block, plain); - for (const text of plain) otherLeaves.push({ text, inFilter: false }); + stringLeaves(block, leaves); } - - return [...filterLeaves, ...otherLeaves]; + return leaves; } /** True when the flow is armed by a record lifecycle event. */ @@ -385,33 +341,18 @@ interface TemplateRoot { } /** - * The text a `loop`'s `config.collection` names its list by: since #19939 a CEL - * value envelope's `source` (`{ dialect: 'cel', source: 'staleCases' }`, the - * spelling the structured `loop` evaluates), and in a source written before - * that, the body of a lone `{…}` token or a bare name ({@link loneTokenBody}; - * every door refuses both now, so reading them only keeps this rule's - * findings on such a flow). The caller resolves it only when it is a bare - * variable name. + * The text a `loop`'s `config.collection` names its list by: a CEL value + * envelope's `source` (`{ dialect: 'cel', source: 'staleCases' }`, the + * spelling the structured `loop` evaluates, #19939). A string there — a lone + * `{…}` token of the retired dialect, or a variable's bare name, which the + * structured `loop` never read — is refused at every door and resolves no + * root here. The caller resolves the source only when it is a bare variable + * name. */ function collectionListText(raw: unknown): string | undefined { - if (raw !== null && typeof raw === 'object' && !Array.isArray(raw)) { - const envelope = raw as AnyRec; - return envelope.dialect === 'cel' && typeof envelope.source === 'string' ? envelope.source.trim() : undefined; - } - return loneTokenBody(raw); -} - -/** - * The body of a string that is EXACTLY one `{…}` token, or the string itself - * when it carries no braces — the two spellings a `loop`'s `config.collection` - * took before #19939 made it a CEL value envelope. - */ -function loneTokenBody(raw: unknown): string | undefined { - if (typeof raw !== 'string') return undefined; - const trimmed = raw.trim(); - if (!trimmed) return undefined; - const braced = /^\{([^{}]+)\}$/.exec(trimmed); - return (braced ? braced[1] : trimmed).trim(); + if (raw === null || typeof raw !== 'object' || Array.isArray(raw)) return undefined; + const envelope = raw as AnyRec; + return envelope.dialect === 'cel' && typeof envelope.source === 'string' ? envelope.source.trim() : undefined; } /** @@ -598,13 +539,11 @@ export function validateFlowTemplatePaths(stack: AnyRec): FlowTemplatePathFindin } // Every node, INCLUDING those nested in try_catch / loop / parallel regions - // (#4380). This rule was not merely blind to them — it was WORSE than - // blind: the recursive string-leaf scan already saw a nested node's tokens - // through its container's `config`, but `collectNodeLeaves` splits `filter` - // only at the top level of the node it is handed, so a nested filter token - // lost its position and the gating #3810 finding silently degraded to a - // warning reported against the wrapping `try_catch`. Walking to the real - // node restores both the severity and the location. + // (#4380). The recursive string-leaf scan already saw a nested node's + // holes through its container's `config`, but judged them against the + // wrapping container: `collectNodeLeaves` reads only the node it is + // handed. Walking to the real node restores the location — and the + // per-node-type `filter` exemption, which a nested CRUD node needs too. const walked = walkFlowNodes(flow, `flows[${flowIndex}]`); // ── Roots 2..n: flow variables a node binds to a record (#17305). The @@ -637,9 +576,9 @@ export function validateFlowTemplatePaths(stack: AnyRec): FlowTemplatePathFindin : `flow "${flowName}" node "${nodeLabel}"`; // Collect templated string leaves from the config-bearing blocks only, - // tagging filter positions when this node type guards its filter (#3810). + // leaving out a CRUD node's filter operands — value slots (#19939). const nodeType = typeof node.type === 'string' ? node.type : ''; - const guarded = FILTER_GUARDED_NODE_TYPES.has(nodeType); + const skipFilter = FILTER_VALUE_SLOT_NODE_TYPES.has(nodeType); // Scan the container's config WITHOUT its region slots: their nodes are // walked in their own right, and leaving them in would report every // nested finding a second time against the container. @@ -647,7 +586,7 @@ export function validateFlowTemplatePaths(stack: AnyRec): FlowTemplatePathFindin localConfig !== undefined && localConfig !== node.config ? ({ ...node, config: localConfig } as AnyRec) : (node as AnyRec); - const leaves = collectNodeLeaves(scanNode, guarded); + const leaves = collectNodeLeaves(scanNode, skipFilter); if (leaves.length === 0) return; // Dedupe references so one repeated typo yields one finding per node. @@ -658,14 +597,14 @@ export function validateFlowTemplatePaths(stack: AnyRec): FlowTemplatePathFindin const seenUnprovisioned = new Set(); for (const leaf of leaves) { - const inFilter = leaf.inFilter; - for (const { root: rootName, rest } of templateRefsIn(leaf.text)) { + for (const { root: rootName, rest } of templateRefsIn(leaf)) { const root = roots.get(rootName); // An unresolvable root — a flow variable nothing binds statically, an - // ambiguous one, `{record.…}` on a flow with no record trigger. + // ambiguous one, `{{ record.… }}` on a flow with no record trigger. if (!root) continue; const objectName = root.objectName; const token = `${rootName}.${rest.join('.')}`; + const hole = `{{ ${token} }}`; const head = rest[0]; const hasSubPath = rest.length > 1; // A trailing numeric segment is an array index (#1872), not a hop. @@ -676,10 +615,7 @@ export function validateFlowTemplatePaths(stack: AnyRec): FlowTemplatePathFindin // [#8340] The head RESOLVES — `IMPLICIT_HEADS` keeps owning that // decision, exactly as before — but on an ADR-0015 `external` trigger // object the platform registers this anchor and stores nothing in it, - // so the interpolator reads an empty value from the flow record. In a - // filter position that is #3810's own failure reached by a second - // route: the token erases the authored condition and `resolveNodeFilter` - // refuses the node at run time. Warning, not error, on both positions: + // so the hole renders an empty value from the flow record. A warning: // unlike a typo (a closed oracle — the field is simply absent) this // pass cannot see whether the remote schema resolves the column. if (root.unprovisionedAnchors.has(head)) { @@ -691,13 +627,8 @@ export function validateFlowTemplatePaths(stack: AnyRec): FlowTemplatePathFindin where, path: nodePath, message: - (inFilter ? `${nodeType} filter references ` : 'template references ') + - `'{${token}}', and ${unprovisionedAnchorCause(objectName, head)} — ` + - (inFilter - ? `the token resolves to nothing on every run, which DROPS the condition from ` + - `the query instead of narrowing it; the node then refuses to run at execution ` + - `time.` - : `the token resolves to an empty string on every run (silently).`), + `template references '${hole}', and ${unprovisionedAnchorCause(objectName, head)} — ` + + `the hole renders an empty string on every run (silently).`, hint: unprovisionedAnchorHint(objectName, head), }); } @@ -707,32 +638,19 @@ export function validateFlowTemplatePaths(stack: AnyRec): FlowTemplatePathFindin if (seenUnknown.has(token)) continue; seenUnknown.add(token); findings.push({ - severity: inFilter ? 'error' : 'warning', + severity: 'warning', rule: FLOW_TEMPLATE_UNKNOWN_FIELD, where, path: nodePath, - message: inFilter - ? `${nodeType} filter references '{${token}}', but '${head}' is not a field on ` + - `object '${objectName}' — the token resolves to nothing, which DROPS the condition from the ` + - `query instead of narrowing it. The node refuses to run at execution time.` - : `template references '{${token}}', but '${head}' is not a field on ` + - `object '${objectName}' — it resolves to an empty string at runtime (silently).`, + message: + `template references '${hole}', but '${head}' is not a field on ` + + `object '${objectName}' — it resolves to an empty string at runtime (silently).`, hint: root.isTrigger - ? inFilter - ? `Check the field name against the object's field definitions (e.g. '{record.full_name}', ` + - `not '{record.full_naem}'); system columns like id/created_at/owner are also addressable. ` + - `This gates the build rather than warning: an absent condition WIDENS the query, so the ` + - `runtime has already decided to refuse this node.` - : `Check the field name against the object's field definitions (e.g. '{record.full_name}', ` + - `not '{record.full_naem}'). System columns like id/created_at/owner are also addressable.` - : inFilter - ? `Check the field name against object '${objectName}'s field definitions — '${rootName}' is ` + - `bound to a '${objectName}' record by this flow; system columns like id/created_at/owner ` + - `are also addressable. This gates the build rather than warning: an absent condition ` + - `WIDENS the query, so the runtime has already decided to refuse this node.` - : `Check the field name against object '${objectName}'s field definitions — '${rootName}' is ` + - `bound to a '${objectName}' record by this flow. System columns like id/created_at/owner ` + - `are also addressable.`, + ? `Check the field name against the object's field definitions (e.g. '{{ record.full_name }}', ` + + `not '{{ record.full_naem }}'). System columns like id/created_at/owner are also addressable.` + : `Check the field name against object '${objectName}'s field definitions — '${rootName}' is ` + + `bound to a '${objectName}' record by this flow. System columns like id/created_at/owner ` + + `are also addressable.`, }); continue; } @@ -743,39 +661,23 @@ export function validateFlowTemplatePaths(stack: AnyRec): FlowTemplatePathFindin if (seenTraversal.has(token)) continue; seenTraversal.add(token); findings.push({ - severity: inFilter ? 'error' : 'warning', + severity: 'warning', rule: FLOW_TEMPLATE_LOOKUP_TRAVERSAL, where, path: nodePath, - message: inFilter - ? `${nodeType} filter references '{${token}}', a cross-object hop through the ` + - `${headType} field '${head}' — the ${root.isTrigger ? 'flow record' : `'${rootName}' record`} ` + - `carries '${head}' as a scalar id, not an ` + - `expanded object, so the token resolves to nothing and the condition is DROPPED from the ` + - `query instead of narrowing it. The node refuses to run at execution time.` - : `template references '{${token}}', a cross-object hop through the ${headType} field ` + - `'${head}' — the ${root.isTrigger ? 'flow record' : `'${rootName}' record`} carries ` + - `'${head}' as a scalar id, not an expanded object, so ` + - `this resolves to an empty string at runtime (silently).`, + message: + `template references '${hole}', a cross-object hop through the ${headType} field ` + + `'${head}' — the ${root.isTrigger ? 'flow record' : `'${rootName}' record`} carries ` + + `'${head}' as a scalar id, not an expanded object, so ` + + `this resolves to an empty string at runtime (silently).`, hint: root.isTrigger - ? inFilter - ? `Opt in to resolve it: add '${head}' to the start node's config.expand and the ` + - `engine re-reads it as the run's identity. Otherwise filter on the foreign-key id directly ` + - `('{record.${head}}'), or project the value via a formula field on '${objectName}'. This ` + - `gates the build rather than warning: an absent condition WIDENS the query.` - : `Opt in to resolve it: add '${head}' to the start node's config.expand and the ` + - `engine re-reads it as the run's identity. Otherwise reference the foreign-key id directly ` + - `('{record.${head}}'), or project the value via a formula field on '${objectName}'.` - : inFilter - ? `A record read into a flow variable is not expanded either — config.expand is the ` + - `START node's opt-in and covers the trigger record alone. Filter on the foreign-key id ` + - `directly ('{${rootName}.${head}}'), project the value via a formula field on ` + - `'${objectName}', or add a get_record node that reads the related record. This gates the ` + - `build rather than warning: an absent condition WIDENS the query.` - : `A record read into a flow variable is not expanded either — config.expand is the ` + - `START node's opt-in and covers the trigger record alone. Reference the foreign-key id ` + - `directly ('{${rootName}.${head}}'), project the value via a formula field on ` + - `'${objectName}', or add a get_record node that reads the related record.`, + ? `Opt in to resolve it: add '${head}' to the start node's config.expand and the ` + + `engine re-reads it as the run's identity. Otherwise reference the foreign-key id directly ` + + `('{{ record.${head} }}'), or project the value via a formula field on '${objectName}'.` + : `A record read into a flow variable is not expanded either — config.expand is the ` + + `START node's opt-in and covers the trigger record alone. Reference the foreign-key id ` + + `directly ('{{ ${rootName}.${head} }}'), project the value via a formula field on ` + + `'${objectName}', or add a get_record node that reads the related record.`, }); } // STRUCTURED_TYPES + any other scalar `.sub` access is left alone: diff --git a/packages/services/service-automation/src/builtin/assignment-value-envelope.test.ts b/packages/services/service-automation/src/builtin/assignment-value-envelope.test.ts index af59f037298..36c9495f3c7 100644 --- a/packages/services/service-automation/src/builtin/assignment-value-envelope.test.ts +++ b/packages/services/service-automation/src/builtin/assignment-value-envelope.test.ts @@ -6,7 +6,7 @@ * half landed in PR #15113. * * Before this, an `assignment` value that was an `{ dialect: 'cel', source }` - * envelope went to `interpolate()`, which recursed into it as a plain object + * envelope went to the 17.x `interpolate()`, which recursed into it as a plain object * and wrote it into the variable VERBATIM — `notify` then rendered * `{"dialect":"cel","source":"…"}` as JSON into a message body. The whole * declared CEL stdlib was unreachable from metadata, because CEL was only ever @@ -44,7 +44,7 @@ import { EVALUATED_EXPRESSION_SOURCE_REQUIRED } from '@objectstack/spec'; import { ExpressionEngine } from '@objectstack/formula'; import { AutomationEngine } from '../engine.js'; import { registerLogicNodes } from './logic-nodes.js'; -import { interpolate } from './template.js'; +import { copyLiteral } from './value-slot-map.js'; /** The thrown error, so a pin can assert its message substance rather than `toThrow()` alone. */ function catchError(fn: () => unknown): Error { @@ -458,7 +458,6 @@ describe('[#19939] where an envelope is literal data, the refusal names a spelli return { ...flow, variables: [{ name: 'name', type: 'text', isInput: true }, ...flow.variables] }; } const RUN = { params: { name: 'Ada' } } as any; - const VARIABLES = new Map([['name', 'Ada']]); /** The envelope source a refusal message names after `lead` — read off the message, never re-spelled. */ function envelopeAfter(message: string, lead: string): string { @@ -483,7 +482,6 @@ describe('[#19939] where an envelope is literal data, the refusal names a spelli engine.registerFlow('assign_flow', literalFlow({ assignments: { greeting: { dialect: 'cel', source } } }, ['greeting'])); const result = await engine.execute('assign_flow', RUN); expect(result.success).toBe(true); - expect(result.output).toEqual({ greeting: interpolate('Hello {name}', VARIABLES, {} as any) }); expect(result.output).toEqual({ greeting: 'Hello Ada' }); }); @@ -518,7 +516,6 @@ describe('[#19939] where an envelope is literal data, the refusal names a spelli engine.registerFlow('assign_flow', literalFlow({ assignments: { o: { dialect: 'cel', source: whole } } }, ['o'])); const result = await engine.execute('assign_flow', RUN); expect(result.success).toBe(true); - expect(result.output).toEqual({ o: interpolate(value, VARIABLES, {} as any) }); expect(result.output).toEqual({ o: { who: 'Ada', meta: { note: 'for Ada' } } }); // Control: the envelope the old remedy named at the nested position is data. @@ -541,6 +538,34 @@ describe('[#19939] where an envelope is literal data, the refusal names a spelli }, ['o'])); const result = await engine.execute('assign_flow', RUN); expect(result.success).toBe(true); - expect(result.output).toEqual({ o: interpolate({ who: '{name}', n: 3 }, VARIABLES, {} as any) }); + expect(result.output).toEqual({ o: { who: 'Ada', n: 3 } }); + }); + + // [#19939 S7] A literal is assigned as a copy of what was written. The + // single-brace interpolator walked every object's own entries, so a `Date` + // written in a code-defined flow was assigned as `{}`. + it.each([ + ['the canonical map', (due: Date) => ({ assignments: { due } })], + ['the legacy `assignments` array', (due: Date) => ({ assignments: [{ variable: 'due', value: due }] })], + ['the legacy bare config', (due: Date) => ({ due })], + ] as const)('%s: a `Date` literal is assigned as the `Date`', async (_label, configOf) => { + const due = new Date('2026-10-13T08:00:00.000Z'); + engine.registerFlow('assign_flow', literalFlow(configOf(due), ['due'])); + const result = await engine.execute('assign_flow', RUN); + expect(result.success, result.error).toBe(true); + expect((result.output as { due: unknown }).due).toBeInstanceOf(Date); + expect(((result.output as { due: Date }).due).toISOString()).toBe('2026-10-13T08:00:00.000Z'); + }); + + it('`copyLiteral` copies arrays and plain objects at every depth, and hands every other value on as it is', () => { + const due = new Date('2026-10-13T08:00:00.000Z'); + const literal = { by: 'Ada', tags: ['a', { due }], none: null }; + const copy = copyLiteral(literal) as typeof literal; + expect(copy).toEqual(literal); + expect(copy).not.toBe(literal); + expect(copy.tags).not.toBe(literal.tags); + expect(copy.tags[1]).not.toBe(literal.tags[1]); + expect((copy.tags[1] as { due: Date }).due).toBe(due); + expect(copyLiteral('{x}')).toBe('{x}'); }); }); diff --git a/packages/services/service-automation/src/builtin/callee-input-value-slots.test.ts b/packages/services/service-automation/src/builtin/callee-input-value-slots.test.ts index 8c730cb185b..ecb603424a4 100644 --- a/packages/services/service-automation/src/builtin/callee-input-value-slots.test.ts +++ b/packages/services/service-automation/src/builtin/callee-input-value-slots.test.ts @@ -6,7 +6,7 @@ * stage; #11182 ruling D; ADR-0032 Decision 2, a computed value is whole-field * CEL). Declared in the expression ledger and evaluated in the same change. * - * Before this, each executor handed its map to `interpolate()` whole: a + * Before this, each executor handed its map to the 17.x `interpolate()` whole: a * `{token}` resolved, and an envelope written there reached the callee as the * object `{ dialect: 'cel', source: '…' }` it spells, with the run reporting * success. Pinned through the shipped `AutomationEngine`: @@ -30,7 +30,6 @@ import { describe, expect, it } from 'vitest'; import { VALUE_SLOT_TEMPLATE_REFUSAL } from '@objectstack/spec/automation'; import { AutomationEngine } from '../engine.js'; import { installBuiltinNodes } from './index.js'; -import { interpolate } from './template.js'; function silentLogger(): any { return { info() {}, warn() {}, error() {}, debug() {}, child() { return silentLogger(); } }; @@ -206,8 +205,8 @@ describe('3. the callee\'s default: the guarded form SUPPLIES `null`, which wins return (run.output as { out: { d: unknown } }).out.d; } + // What the 17.x interpolator handed for `{ d: '{nope}' }` was `{ d: undefined }`. it('what the template did: a whole token that resolved to nothing handed `undefined`, so the default applied', async () => { - expect(interpolate({ d: '{nope}' }, new Map(), {} as any)).toEqual({ d: undefined }); const engine = makeEngine(); engine.registerFlow('child', echoChild('child', [{ name: 'd', defaultValue: 'dflt' }])); const run = await engine.execute('child', { params: { d: undefined } } as any); @@ -241,9 +240,8 @@ describe('3. the callee\'s default: the guarded form SUPPLIES `null`, which wins id: 'n', type: 'script', config: { function: 'echo', inputs: { k: GUARDED } }, }, PARENT_VARIABLES)); expect((await engine.execute('parent', {} as any)).success).toBe(true); + // (The 17.x interpolator handed `{ k: '{nope}' }` on as `{ k: undefined }`.) expect(seen).toEqual([{ k: null }]); - expect('k' in interpolate({ k: '{nope}' }, new Map(), {} as any)).toBe(true); - expect(interpolate({ k: '{nope}' }, new Map(), {} as any).k).toBeUndefined(); }); it('the registration refusal says so, at the position', () => { diff --git a/packages/services/service-automation/src/builtin/collection-value-slots.test.ts b/packages/services/service-automation/src/builtin/collection-value-slots.test.ts index 2c9ac5697a4..cc97633ca78 100644 --- a/packages/services/service-automation/src/builtin/collection-value-slots.test.ts +++ b/packages/services/service-automation/src/builtin/collection-value-slots.test.ts @@ -6,10 +6,10 @@ * D; ADR-0032 Decision 2, a computed value is whole-field CEL). Declared in the * expression ledger and evaluated in the same change. * - * Before this, each executor ran a string `collection` through `interpolate()`: - * a whole `{token}` handed back the variable's value, and the contract also - * declared a variable's bare name there — which the structured `loop` and - * `map` never read: `interpolate('rows')` returns `'rows'`, the fallback to a + * Before this, each executor ran a string `collection` through the 17.x + * `interpolate()`: a whole `{token}` handed back the variable's value, and the + * contract also declared a variable's bare name there — which the structured + * `loop` and `map` never read: `interpolate('rows')` returned `'rows'`, the fallback to a * variable lookup fired only on `null`, and every run failed at the node with * "did not resolve to an array" (measured on the base through this engine). * Pinned through the shipped `AutomationEngine`: diff --git a/packages/services/service-automation/src/builtin/crud-fields-value-envelope.test.ts b/packages/services/service-automation/src/builtin/crud-fields-value-envelope.test.ts index 1633921810f..dd62114e435 100644 --- a/packages/services/service-automation/src/builtin/crud-fields-value-envelope.test.ts +++ b/packages/services/service-automation/src/builtin/crud-fields-value-envelope.test.ts @@ -6,7 +6,7 @@ * #19938 makes (maintainer ruling B: one change, so the slot is never declared * without being evaluated). * - * Before this, both executors handed the whole `fields` map to `interpolate()`, + * Before this, both executors handed the whole `fields` map to the 17.x `interpolate()`, * which recursed into an envelope as plain data: a text or JSON column received * the literal `{"dialect":"cel","source":"…"}` with the run reporting success, * and a number column was refused by the data engine. Measured on the base @@ -17,20 +17,22 @@ * 1. **Evaluate** — a valid envelope in `fields.*` is evaluated by the same * engine call the `assignment` executor makes, and its value is written, * type kept. - * 2. **Preserve** — every literal writes exactly what the old whole-map - * `interpolate()` wrote. (A `{…}` template no longer reaches a write: - * #19939, ruling D's v18 half, refuses it — pinned below, with the CEL - * spelling that writes the same value.) + * 2. **Preserve** — every literal writes exactly what was written, as the + * old whole-map `interpolate()` wrote it — and, since #19939 S7 deleted + * that interpolator, a `Date` as the `Date` it is, where it wrote `{}`. + * (A `{…}` template no longer reaches a write: #19939, ruling D's v18 + * half, refuses it — pinned below, with the CEL spelling that writes the + * same value.) * 3. **Refuse at registration** — a malformed envelope stops the flow * registering, located at `config.fields.` and led by the * slot-neutral sentence; the evaluator refuses the same set. * 4. **Fault loudly** — an envelope that parses but cannot evaluate on the * live values fails the run with its source, and writes nothing. - * 5. **The date macros' parity** (#19939 pass 3) — the live cross-dialect - * pin: for each date macro, the 17.x interpolator and the CEL envelope the - * refusal names run over the SAME instants, and the store receives the - * same bytes from both; the edges where they part are pinned as the - * refusal names them. + * 5. **The date macros' parity** (#19939 pass 3) — for each date macro, the + * bytes the 17.x macro wrote ({@link macroBytes}, its UTC calendar + * arithmetic, frozen here since #19939 S7 deleted the interpolator) and + * what the CEL envelope the refusal names writes, over the SAME instants; + * the edges where they part are pinned as the refusal names them. */ import { afterEach, describe, it, expect, vi } from 'vitest'; @@ -38,7 +40,6 @@ import { ObjectQL } from '@objectstack/objectql'; import { VALUE_ENVELOPE_REFUSAL, VALUE_SLOT_TEMPLATE_REFUSAL, valueSlotTemplateRefusals } from '@objectstack/spec/automation'; import { AutomationEngine } from '../engine.js'; import { registerCrudNodes } from './crud-nodes.js'; -import { interpolate, interpolateString } from './template.js'; function makeLogger(): any { const l: any = { info() {}, warn() {}, error() {}, debug() {}, trace() {}, fatal() {} }; @@ -79,6 +80,7 @@ async function makeStack() { total: { name: 'total', type: 'number' }, subject: { name: 'subject', type: 'text' }, payload: { name: 'payload', type: 'json' }, + due: { name: 'due', type: 'datetime' }, }, } as any, 'test'); const automation = new AutomationEngine(logger); @@ -168,10 +170,10 @@ describe.each(NODE_TYPES)('%s `fields.*` — a CEL value envelope is EVALUATED ( }); it('this is a CHANGE: the same field used to be written as the envelope object', async () => { - // The pre-change behaviour, reproduced through the old code path itself: - // `interpolate()` over the whole map recursed into the envelope as data. + // The pre-change behaviour (measured through the 17.x code path itself): + // `interpolate()` over the whole map recursed into the envelope as data + // and wrote `{ payload: envelope }`. const envelope = { dialect: 'cel', source: 'upper(name)' }; - expect(interpolate({ payload: envelope }, new Map(Object.entries(PARAMS)), {} as any)).toEqual({ payload: envelope }); const { automation, writes } = await makeStack(); automation.registerFlow('price_quote', writeFlow(nodeType, { payload: envelope })); @@ -182,7 +184,7 @@ describe.each(NODE_TYPES)('%s `fields.*` — a CEL value envelope is EVALUATED ( }); describe.each(NODE_TYPES)('%s `fields.*` — every literal writes exactly what it wrote before', (nodeType) => { - it('literals, arrays and nested envelope-shaped JSON: byte-identical to the whole-map `interpolate()`', async () => { + it('literals, arrays and nested envelope-shaped JSON: written as they are', async () => { const fields = { subject: '2026-10-17', // an ISO date, as it is (a date macro is refused, #19939) total: 42, @@ -193,17 +195,29 @@ describe.each(NODE_TYPES)('%s `fields.*` — every literal writes exactly what i weird: { dialect: 1 }, }, }; - const before = interpolate(fields, new Map(Object.entries(PARAMS)), {} as any); - const { automation, writes } = await makeStack(); automation.registerFlow('price_quote', writeFlow(nodeType, fields)); const res = await run(automation); expect(res.success, res.error).toBe(true); const written = { ...writes[0]!.data }; for (const k of Object.keys(written)) if (!(k in fields)) delete written[k]; // platform stamps - expect(written).toEqual(before); + expect(written).toEqual(fields); expect((written.payload as { inner: unknown }).inner, 'a nested envelope is DATA').toEqual({ dialect: 'cel', source: 'price * 2' }); }); + + // [#19939 S7] The single-brace interpolator walked every object's own + // entries, so a `Date` written in a code-defined flow reached the store as + // `{}`. A literal is handed on as a copy of what was written now. + it('a `Date` literal is written as that instant, never as `{}`', async () => { + const due = new Date('2026-10-13T08:00:00.000Z'); + const { automation, writes } = await makeStack(); + automation.registerFlow('price_quote', writeFlow(nodeType, { due })); + const res = await run(automation); + expect(res.success, res.error).toBe(true); + const written = writes0(writes).due; + expect(written).not.toEqual({}); + expect(new Date(written as string | Date).toISOString()).toBe('2026-10-13T08:00:00.000Z'); + }); }); /** @@ -250,7 +264,8 @@ describe.each(NODE_TYPES)('%s `fields.*` — the retired `{…}` template dialec }); it('the CEL spelling the refusal names writes the value the template wrote', async () => { - const before = interpolate({ total: '{price}', subject: 'Quote for {name}' }, new Map(Object.entries(PARAMS)), {} as any); + // What the 17.x interpolator wrote for `{ total: '{price}', subject: 'Quote for {name}' }`. + const before = { total: PARAMS.price, subject: `Quote for ${PARAMS.name}` }; const { automation, writes } = await makeStack(); automation.registerFlow('price_quote', writeFlow(nodeType, { total: { dialect: 'cel', source: 'price' }, @@ -271,13 +286,13 @@ describe.each(NODE_TYPES)('%s `fields.*` — the retired `{…}` template dialec expect(() => automation.registerFlow('price_quote', writeFlow(nodeType, { subject: '{$User.Id}' }))) .toThrow("{ dialect: 'cel', source: 'current_user.id' }"); - const before = interpolate({ subject: '{$User.Id}' }, new Map(), { userId: 'u1' } as any); + // What the 17.x interpolator wrote for `{ subject: '{$User.Id}' }` in a run with a user. + const before = { subject: 'u1' }; const withUser = await makeStack(); withUser.automation.registerFlow('price_quote', writeFlow(nodeType, { subject: { dialect: 'cel', source: 'current_user.id' } })); const res = await run(withUser.automation); expect(res.success, res.error).toBe(true); expect(writes0(withUser.writes).subject).toBe(before.subject); - expect(before.subject).toBe('u1'); // A user-less run writes data only under an explicit `runAs: 'system'` (ADR-0049). const userless = await makeStack(); @@ -287,8 +302,7 @@ describe.each(NODE_TYPES)('%s `fields.*` — the retired `{…}` template dialec }); const res2 = await userless.automation.execute('price_quote', { params: PARAMS } as any); expect(res2.success, res2.error).toBe(true); - expect(interpolate({ subject: '{$User.Id}' }, new Map(), {} as any).subject).toBeUndefined(); - // The guarded form sends `null` — on `update_record` that clears the stored value. + // (The template wrote nothing there: `subject` was `undefined`.) The guarded form sends `null` — on `update_record` that clears the stored value. expect(writes0(userless.writes)).toHaveProperty('subject', null); }); @@ -306,13 +320,13 @@ describe.each(NODE_TYPES)('%s `fields.*` — the retired `{…}` template dialec expect(source).toBe("{'note': 'for ' + name}"); expect(refusal!.message).not.toContain(`{ dialect: 'cel', source: "'for ' + name" }`); - const before = interpolate(fields, new Map(Object.entries(PARAMS)), {} as any); + // What the 17.x interpolator wrote for `fields`. + const before = { payload: { note: 'for ada' } }; const remedied = await makeStack(); remedied.automation.registerFlow('price_quote', writeFlow(nodeType, { payload: { dialect: 'cel', source } })); const res = await run(remedied.automation); expect(res.success, res.error).toBe(true); expect(writes0(remedied.writes).payload).toEqual(before.payload); - expect(before.payload).toEqual({ note: 'for ada' }); // Why the remedy names no envelope at the nested position: one there is data. const nestedEnvelope = await makeStack(); @@ -381,12 +395,12 @@ describe.each(NODE_TYPES)('%s `fields.*` — a value that cannot be computed fai * [#19939 pass 3] **The live cross-dialect parity pin.** The date macros are * refused in a value slot with their CEL string form (`isoDate` / `isoDatetime` * in `@objectstack/formula`'s stdlib, UTC). `stdlib-timestamp-text.test.ts` - * there measures the CEL half against hand-written bytes; THIS pin is the one - * that runs both dialects live, in one test, over the same instants: the 17.x - * interpolator (`interpolateString`, the template's own resolver) and the - * envelope the spec's refusal names, evaluated by the engine and written - * through a real ObjectQL engine to the store. `Date` is frozen per instant, - * so both read the same `now`. + * there measures the CEL half against hand-written bytes; THIS pin sets, over + * the same instants, what the 17.x date macro wrote ({@link macroBytes}, the + * interpolator's UTC calendar arithmetic, frozen here since #19939 S7 deleted + * that interpolator) beside the envelope the spec's refusal names, evaluated + * by the engine and written through a real ObjectQL engine to the store. + * `Date` is frozen per instant, so both read the same `now`. * * The instants are the cells where a calendar mistake shows: a spring-forward * and a fall-back DST transition day, the last millisecond of a month, a year @@ -442,36 +456,40 @@ describe('the date macros — the envelope each refusal names writes the bytes t return writes0(writes).subject; } - /** What the 17.x interpolator wrote for `authored` at `instant`. */ - function templated(authored: string, instant: string): unknown { - vi.useFakeTimers({ toFake: ['Date'] }); - vi.setSystemTime(new Date(instant)); - return interpolateString(authored, new Map(Object.entries(VARS)), CONTEXT); + /** + * What a 17.x date macro wrote at `instant` — `resolveToken`'s date branch, + * frozen: the offset in whole UTC days, a fraction truncated in the day sum + * (`setUTCDate`), and `NOW()` as the full ISO text, `TODAY()` its first ten + * characters. A non-numeric offset the template did not find as a variable + * read as 0 (`{TODAY() + 3d}`). + */ + function macroBytes(fn: 'NOW' | 'TODAY', offset: number, instant: string): string { + const now = new Date(instant); + if (offset) now.setUTCDate(now.getUTCDate() + offset); + return fn === 'NOW' ? now.toISOString() : now.toISOString().slice(0, 10); } it.each([ - ['{TODAY()}', 'isoDate(today())'], - ['{TODAY() + 7}', 'isoDate(daysFromNow(7))'], - ['{TODAY() - 3}', 'isoDate(daysAgo(3))'], - ['{NOW()}', 'isoDatetime(now())'], - ['{NOW() + 2}', 'isoDatetime(addDays(now(), 2))'], - ['{NOW() - 1}', 'isoDatetime(addDays(now(), -1))'], - ['{TODAY() + days}', 'isoDate(addDays(today(), days))'], - ['{TODAY() - days}', 'isoDate(addDays(today(), -days))'], - ['{NOW() + days}', 'isoDatetime(addDays(now(), days))'], - ['{TODAY() + 1.5}', 'isoDate(addDays(today(), 1.5))'], - ['{TODAY() + 3d}', 'isoDate(today())'], - ['Due {TODAY() + 7} for {name}', "'Due ' + isoDate(daysFromNow(7)) + ' for ' + name"], - ['Due {TODAY()} by {$User.Id}', "'Due ' + isoDate(today()) + ' by ' + current_user.id"], - ])('%s → %s', async (authored, source) => { + ['{TODAY()}', 'isoDate(today())', (at: string) => macroBytes('TODAY', 0, at)], + ['{TODAY() + 7}', 'isoDate(daysFromNow(7))', (at: string) => macroBytes('TODAY', 7, at)], + ['{TODAY() - 3}', 'isoDate(daysAgo(3))', (at: string) => macroBytes('TODAY', -3, at)], + ['{NOW()}', 'isoDatetime(now())', (at: string) => macroBytes('NOW', 0, at)], + ['{NOW() + 2}', 'isoDatetime(addDays(now(), 2))', (at: string) => macroBytes('NOW', 2, at)], + ['{NOW() - 1}', 'isoDatetime(addDays(now(), -1))', (at: string) => macroBytes('NOW', -1, at)], + ['{TODAY() + days}', 'isoDate(addDays(today(), days))', (at: string) => macroBytes('TODAY', VARS.days, at)], + ['{TODAY() - days}', 'isoDate(addDays(today(), -days))', (at: string) => macroBytes('TODAY', -VARS.days, at)], + ['{NOW() + days}', 'isoDatetime(addDays(now(), days))', (at: string) => macroBytes('NOW', VARS.days, at)], + ['{TODAY() + 1.5}', 'isoDate(addDays(today(), 1.5))', (at: string) => macroBytes('TODAY', 1.5, at)], + ['{TODAY() + 3d}', 'isoDate(today())', (at: string) => macroBytes('TODAY', 0, at)], + ['Due {TODAY() + 7} for {name}', "'Due ' + isoDate(daysFromNow(7)) + ' for ' + name", (at: string) => `Due ${macroBytes('TODAY', 7, at)} for ${VARS.name}`], + ['Due {TODAY()} by {$User.Id}', "'Due ' + isoDate(today()) + ' by ' + current_user.id", (at: string) => `Due ${macroBytes('TODAY', 0, at)} by ${CONTEXT.userId}`], + ] as const)('%s → %s', async (authored, source, template) => { // The parity is asked of the spelling the judge PRINTS, so a remedy that // drifted from the template fails on the written bytes; the table only // names which spelling that is. const remedy = remedyOf(authored); for (const instant of INSTANTS) { - const template = templated(authored, instant); - expect(typeof template, `${authored} @ ${instant}`).toBe('string'); - expect(await written({ dialect: 'cel', source: remedy }, instant), `${authored} → ${remedy} @ ${instant}`).toBe(template); + expect(await written({ dialect: 'cel', source: remedy }, instant), `${authored} → ${remedy} @ ${instant}`).toBe(template(instant)); } expect(remedy).toBe(source); }); @@ -482,10 +500,10 @@ describe('the date macros — the envelope each refusal names writes the bytes t const source = remedyOf(authored); expect(source).toBe('isoDate(addDays(today(), -1.5))'); // Past the day of the month the offset exceeds: the template moved 2 days back, `addDays` 1. - expect(templated(authored, '2026-03-08T09:30:00.000Z')).toBe('2026-03-06'); + expect(macroBytes('TODAY', -1.5, '2026-03-08T09:30:00.000Z')).toBe('2026-03-06'); expect(await written({ dialect: 'cel', source }, '2026-03-08T09:30:00.000Z')).toBe('2026-03-07'); // On the 1st the template's sum truncates to 0, the last day of the month before: the two agree. - expect(templated(authored, '2026-03-01T09:30:00.000Z')).toBe('2026-02-28'); + expect(macroBytes('TODAY', -1.5, '2026-03-01T09:30:00.000Z')).toBe('2026-02-28'); expect(await written({ dialect: 'cel', source }, '2026-03-01T09:30:00.000Z')).toBe('2026-02-28'); }); @@ -501,8 +519,7 @@ describe('the date macros — the envelope each refusal names writes the bytes t vi.useFakeTimers({ toFake: ['Date'] }); vi.setSystemTime(new Date('2026-03-08T09:30:00.000Z')); for (const params of [{ name: 'ada', days: 'soon' }, { name: 'ada' }]) { - // The template moved 0 days, silently. - expect(interpolateString('{TODAY() + days}', new Map(Object.entries(params)), CONTEXT)).toBe('2026-03-08'); + // The template moved 0 days, silently: it wrote '2026-03-08'. const { automation, writes } = await makeStack(); automation.registerFlow('macro_parity', macroFlow({ dialect: 'cel', source })); const res = await automation.execute('macro_parity', { userId: 'u1', params } as any); diff --git a/packages/services/service-automation/src/builtin/logic-nodes.ts b/packages/services/service-automation/src/builtin/logic-nodes.ts index 9e11daff3bc..4565cf51492 100644 --- a/packages/services/service-automation/src/builtin/logic-nodes.ts +++ b/packages/services/service-automation/src/builtin/logic-nodes.ts @@ -4,7 +4,7 @@ import type { PluginContext } from '@objectstack/core'; import { defineActionDescriptor, flowNodeValueTemplateRefusals, isExpressionEnvelopeShaped } from '@objectstack/spec/automation'; import { DEFAULT_BRANCH_LABEL, type AutomationEngine } from '../engine.js'; import { refuseNode } from '../guard-refusal.js'; -import { interpolate } from './template.js'; +import { copyLiteral } from './value-slot-map.js'; /** * Logic built-in nodes — decision / assignment. @@ -106,11 +106,11 @@ export function registerLogicNodes(engine: AutomationEngine, ctx: PluginContext) // any variable is set, by the one judge `registerFlow` and `objectstack // validate` call (`flowNodeValueTemplateRefusals`) — so the legacy // shapes are no way around it, and a flow that registered cannot be - // refused here. What reaches `interpolate()` below carries no token — - // the retirement keeps none; a date macro is refused with its CEL - // string form — so there it is the identity. The run user is - // `current_user` in a CEL envelope, bound from the `context` this - // executor passes to the evaluator. + // refused here. A literal is assigned as it is, a copy of it + // (`copyLiteral`) — the single-brace resolver that used to walk it is + // deleted (ADR-0032 Sequencing 6). The run user is `current_user` in + // a CEL envelope, bound from the `context` this executor passes to the + // evaluator. // // [#15137] …with ONE exception, and only in the canonical map: a value // that is envelope-shaped (`isExpressionEnvelopeShaped` — a plain object @@ -202,7 +202,7 @@ export function registerLogicNodes(engine: AutomationEngine, ctx: PluginContext) variables.set(key, engine.evaluateValueEnvelope(value, variables, `assignments.${key}`, context)); continue; } - variables.set(key, interpolate(value, variables, context)); + variables.set(key, copyLiteral(value)); } return { success: true }; }, diff --git a/packages/services/service-automation/src/builtin/screen-value-slots.test.ts b/packages/services/service-automation/src/builtin/screen-value-slots.test.ts index 15115197ce4..9da20686b91 100644 --- a/packages/services/service-automation/src/builtin/screen-value-slots.test.ts +++ b/packages/services/service-automation/src/builtin/screen-value-slots.test.ts @@ -7,7 +7,7 @@ * ruling D; ADR-0032 Decision 2, a computed value is whole-field CEL). * Declared in the expression ledger and evaluated in the same change. * - * Before this, the executor handed each to `interpolate()`: a `{token}` + * Before this, the executor handed each to the 17.x `interpolate()`: a `{token}` * resolved, an envelope written at `defaults.*` or `defaultValue` went on the * wire as the object `{ dialect: 'cel', source: '…' }` it spells, and * `recordId` was `String(resolved)` of whatever the token found. Pinned @@ -32,7 +32,6 @@ import { describe, expect, it } from 'vitest'; import { VALUE_ENVELOPE_REFUSAL, VALUE_SLOT_TEMPLATE_REFUSAL } from '@objectstack/spec/automation'; import { AutomationEngine } from '../engine.js'; import { installBuiltinNodes } from './index.js'; -import { interpolate } from './template.js'; function silentLogger(): any { return { info() {}, warn() {}, error() {}, debug() {}, child() { return silentLogger(); } }; @@ -182,8 +181,8 @@ describe('3. absent and `null` — what each position serves, and where the run const GUARDED = CEL('has(vars.nope) ? vars.nope : null'); it('what the template did: a whole token that resolved to nothing served nothing — the key leaves the wire', () => { - const served = interpolate({ account: '{nope}' }, new Map(), {} as any) as Record; - expect(served).toEqual({ account: undefined }); + // The 17.x interpolator served `{ account: '{nope}' }` as `{ account: undefined }`. + const served: Record = { account: undefined }; expect(JSON.parse(JSON.stringify(served))).toEqual({}); }); @@ -228,8 +227,8 @@ describe('3. absent and `null` — what each position serves, and where the run }); it('recordId: the template\'s `String(resolved)` is gone — a record the token found is no id', async () => { - // The 17.x reader stringified whatever the token resolved to. - expect(String(interpolate('{rec}', new Map([['rec', REC]]), {} as any))).toBe('[object Object]'); + // The 17.x reader stringified whatever the token resolved to: `[object Object]` for a record. + expect(String(REC)).toBe('[object Object]'); const run = await serve({ objectName: 'account', mode: 'edit', recordId: CEL('rec') }); expect(run.success).toBe(false); }); diff --git a/packages/services/service-automation/src/builtin/template-array-index.test.ts b/packages/services/service-automation/src/builtin/template-array-index.test.ts deleted file mode 100644 index 92c94fe3465..00000000000 --- a/packages/services/service-automation/src/builtin/template-array-index.test.ts +++ /dev/null @@ -1,32 +0,0 @@ -// Copyright (c) 2025 ObjectStack. Licensed under the Apache-2.0 license. - -/** - * #1872 — `{var.path.N}` numeric segments index into arrays, so a multi-value - * lookup (array column) can be referenced positionally in flow interpolation. - */ -import { describe, it, expect } from 'vitest'; -import { interpolateString } from './template.js'; - -const ctx = {} as any; -const vars = new Map([ - ['record', { items: ['a', 'b', 'c'], target_channels: ['ch_1', 'ch_2'] }], - ['list', ['x', 'y']], -]); - -describe('interpolate array-index segments (#1872)', () => { - it('resolves a single-token array index, preserving type', () => { - expect(interpolateString('{record.items.0}', vars, ctx)).toBe('a'); - expect(interpolateString('{record.items.2}', vars, ctx)).toBe('c'); - expect(interpolateString('{record.target_channels.0}', vars, ctx)).toBe('ch_1'); - expect(interpolateString('{list.1}', vars, ctx)).toBe('y'); - }); - - it('resolves an array index embedded in a larger string', () => { - expect(interpolateString('first={record.items.0};second={record.items.1}', vars, ctx)).toBe('first=a;second=b'); - }); - - it('out-of-range index yields undefined (single) / empty (embedded)', () => { - expect(interpolateString('{record.items.9}', vars, ctx)).toBeUndefined(); - expect(interpolateString('x={record.items.9}', vars, ctx)).toBe('x='); - }); -}); diff --git a/packages/services/service-automation/src/builtin/template-date-offset-dst.test.ts b/packages/services/service-automation/src/builtin/template-date-offset-dst.test.ts deleted file mode 100644 index f33aec4f2eb..00000000000 --- a/packages/services/service-automation/src/builtin/template-date-offset-dst.test.ts +++ /dev/null @@ -1,325 +0,0 @@ -// Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license. - -/** - * #14852 — the `{TODAY() +/- n}` / `{NOW() +/- n}` offset branch resolves on - * ONE calendar (UTC), the same one the bare forms already render on. - * - * ## Why this file cannot be written to run only at `TZ=UTC` - * - * The two spellings — local `setDate(getDate() + n)` and UTC - * `setUTCDate(getUTCDate() + n)` — are behaviourally INDISTINGUISHABLE at - * `TZ=UTC`, which is precisely why nothing in CI ever went red on the defect - * and why it shipped. Every case below therefore fakes BOTH halves of the - * environment: a DST-observing zone (`process.env.TZ`, re-read by V8 on the - * next `Date` operation) AND an instant whose day shift crosses that zone's - * own transition. - * - * ## The mechanism, stated once - * - * `setDate` preserves WALL-CLOCK time, so shifting the local calendar by n - * days moves the INSTANT by exactly n x 24h only while every local day in the - * window is 24 hours long. Across a spring-forward that window is 23 hours; - * across a fall-back, 25. The rendering is `toISOString()` — UTC. So a flip - * needs TWO conditions AT ONCE: - * - * 1. the local day shift straddles the transition -> the instant moves - * 23h or 25h instead of n x 24h; and - * 2. that one hour of slack crosses a UTC midnight -> the rendered DAY, not - * merely the instant, comes out wrong. - * - * Condition (2) is what pins the instants below to the UTC hour [00:00, 01:00) - * for a forward offset and [23:00, 24:00) for a backward one: those are the - * only hours where one hour of slack changes which UTC day you land on. - * Condition (1) is what pins the DAY to each zone's own transition. Miss - * either and the cell is GREEN against the broken code — a single-point sweep - * is exactly what let the identical two-calendar shape sit unnoticed in a - * hotcrm test helper for months (`objectstack-ai/hotcrm#1462`). - * - * NOTE the direction is not one-signed: spring-forward renders a day EARLY - * (23h falls short of the midnight it had to cross), fall-back renders a day - * LATE (25h overshoots one). Both are pinned. - * - * ## The inline control is load-bearing - * - * Each cell also evaluates the OLD mixed spelling directly and asserts it - * DISAGREES with the truth. Without that, a green run would be ambiguous - * between "the fix works" and "these instants are not actually in a transition - * window" — the second being the failure mode that hid this bug. With it, the - * file proves its own instants are live before it credits the fix. - * - * ## Deliberately NOT pinned here - * - * Whether these tokens should be timezone-AWARE at all is a separate and - * larger question (#14852 explicitly does not propose it). The bare - * `{TODAY()}` resolves to the UTC day, and the controls below hold it there in - * every zone; this file only makes the offset branch AGREE with the bare one. - * - * ## Why the clocked window is kept empty of setup (#19768) - * - * Every case here is SYNCHRONOUS and performs no I/O, no module resolution and - * no `await`. A synchronous body cannot be interrupted by the per-test budget's - * own timer, so vitest's verdict on it is a POST-HOC `performance.now()` - * comparison taken when the body returns (`runWithTimeout`, @vitest/runner - * 4.1.11). On this file "Test timed out in 5000ms" can therefore only ever mean - * "the wall clock advanced 5s while this body ran" -- never "the test waited - * for something". - * - * Measured on one container, per case: the whole `at()` window is 0.15-0.38ms - * cold, 2.34ms for the first case, which also pays the JIT. Against a 5000ms - * budget that is ~13,000x of headroom, so no construct in this file explains - * the CI timeout that filed the card. A whole-process stall does, and one of - * that shape reproduces here: under six concurrent `find node_modules -type f` - * loops, an operation whose median is 0.005ms drew 4.058ms -- an 800x stretch - * of a window that touches no filesystem at all. - * - * A stall cannot be repaired from inside a test file. What can be, and is, is - * the size of the exposed window: the budget should be spent on the calendar - * arithmetic and on nothing else. - * - * - The fake `Date` is installed ONCE for the file, not once per `at()`. - * Install + uninstall measured 0.07-0.25ms per pair, about 57% of the warm - * window; the aggregate cases below call `at()` 11-14 times each, so that - * pair was the dominant cost of this file's heaviest windows. - * - Every zone the file enters is entered ONCE at module load, before any - * clocked window exists. First use of a zone costs ~0.011ms more than a - * repeat at the median (warm code, 30 zones) and carries a tail the repeat - * does not (max 0.679ms vs 0.024ms). Small -- but it is LOADING, and a - * clocked window measures behaviour, never loading. - * - * NOTE what is deliberately NOT done: the budget is not raised, no case is - * skipped, quarantined or made flaky-tolerant, and no retry is configured. The - * work needs a fraction of a millisecond, so a bigger number would record a - * property of the machine as if it were a property of this test. - */ - -import { describe, it, expect, vi, beforeAll, afterAll } from 'vitest'; -import { interpolateString } from './template.js'; - -const ctx = {} as any; - -function tpl(expr: string, vars: Record = {}): unknown { - return interpolateString(`{${expr}}`, new Map(Object.entries(vars)), ctx); -} - -const REAL_TZ = process.env.TZ; - -/** - * Put the process back on the timezone it started on. `at()` calls this in a - * `finally`, so a failing assertion leaves no mutated `TZ` behind, and the last - * fence in this file asserts the invariant directly. - */ -function restoreTz(): void { - if (REAL_TZ === undefined) delete process.env.TZ; - else process.env.TZ = REAL_TZ; -} - -/** - * Enter every zone this file uses ONCE here, at module load, so that no clocked - * window is ever the FIRST user of a zone (see the header). A LOCAL-calendar - * read is what forces the zone to be resolved -- `toISOString()` alone is UTC - * and would warm nothing. - */ -function warmZoneData(zones: readonly string[]): void { - for (const zone of zones) { - process.env.TZ = zone; - const probe = new Date('2026-06-15T12:00:00Z'); - probe.setDate(probe.getDate() + 1); - } - restoreTz(); -} - -/** - * The fake `Date` belongs to the FILE, not to a case: installing and removing - * it inside `at()` put 11-14 install/uninstall pairs inside the aggregate - * cases' clocked windows and bought nothing, since every case in this file - * wants exactly the same fake. `setSystemTime` stays per case -- it is the part - * that carries the cell's instant. - */ -beforeAll(() => { - vi.useFakeTimers({ toFake: ['Date'] }); -}); - -afterAll(() => { - vi.useRealTimers(); -}); - -/** Run `fn` with the process on `zone` and the clock frozen at `instant`. */ -function at(zone: string, instant: string, fn: () => T): T { - process.env.TZ = zone; - vi.setSystemTime(new Date(instant)); - try { - return fn(); - } finally { - restoreTz(); - } -} - -/** The defect, spelled out: day arithmetic on the LOCAL calendar, rendered on UTC. */ -function mixedCalendarSpelling(instant: string, n: number): string { - const d = new Date(instant); - d.setDate(d.getDate() + n); - return d.toISOString().slice(0, 10); -} - -/** - * The truth, computed from NEITHER spelling: the instant plus n x 24h, in UTC. - * `{TODAY() + n}` means "n days after the UTC day it is now", and a UTC day is - * always 24 hours — so this is the definition, not a second implementation. - */ -function utcDayShift(instant: string, n: number): string { - return new Date(Date.parse(instant) + n * 86_400_000).toISOString().slice(0, 10); -} - -interface Cell { - zone: string; - /** Frozen clock, always written in UTC. */ - instant: string; - offset: number; - /** The same instant as local wall clock, so the window is readable. */ - local: string; - kind: 'spring-forward' | 'fall-back'; -} - -/** - * Red cells — every one MEASURED, not guessed: each is an instant at which the - * mixed spelling actually disagrees with the truth in that zone, taken from a - * 34-zone x 30-minute sweep of 2026. Both hemispheres, both transition - * directions, and three zones whose standard offset is not a whole hour - * (St_Johns -03:30, Chatham +12:45, Adelaide +09:30 via Sydney's sibling rule) - * so a whole-hour assumption cannot hide in the fix. - */ -const DST_CELLS: Cell[] = [ - { zone: 'America/New_York', instant: '2026-03-08T00:30:00Z', offset: 1, local: 'Sat 2026-03-07 19:30 EST', kind: 'spring-forward' }, - { zone: 'America/New_York', instant: '2026-03-08T23:30:00Z', offset: -1, local: 'Sun 2026-03-08 19:30 EDT', kind: 'spring-forward' }, - { zone: 'America/New_York', instant: '2026-10-31T23:30:00Z', offset: 1, local: 'Sat 2026-10-31 19:30 EDT', kind: 'fall-back' }, - { zone: 'America/New_York', instant: '2026-11-02T00:30:00Z', offset: -1, local: 'Sun 2026-11-01 19:30 EST', kind: 'fall-back' }, - { zone: 'America/Los_Angeles', instant: '2026-03-08T00:30:00Z', offset: 1, local: 'Sat 2026-03-07 16:30 PST', kind: 'spring-forward' }, - { zone: 'America/St_Johns', instant: '2026-03-08T00:30:00Z', offset: 1, local: 'Sat 2026-03-07 21:00 NST', kind: 'spring-forward' }, - { zone: 'Europe/London', instant: '2026-03-29T00:30:00Z', offset: 1, local: 'Sun 2026-03-29 00:30 GMT', kind: 'spring-forward' }, - { zone: 'Europe/Berlin', instant: '2026-03-29T00:30:00Z', offset: 1, local: 'Sun 2026-03-29 01:30 CET', kind: 'spring-forward' }, - { zone: 'Asia/Jerusalem', instant: '2026-03-27T23:30:00Z', offset: -1, local: 'Sat 2026-03-28 02:30 IDT', kind: 'spring-forward' }, - { zone: 'Australia/Sydney', instant: '2026-10-03T00:30:00Z', offset: 1, local: 'Sat 2026-10-03 10:30 AEST', kind: 'spring-forward' }, - { zone: 'Australia/Adelaide', instant: '2026-10-03T00:30:00Z', offset: 1, local: 'Sat 2026-10-03 10:00 ACST', kind: 'spring-forward' }, - { zone: 'Pacific/Auckland', instant: '2026-09-26T00:30:00Z', offset: 1, local: 'Sat 2026-09-26 12:30 NZST', kind: 'spring-forward' }, - { zone: 'Pacific/Chatham', instant: '2026-09-26T00:30:00Z', offset: 1, local: 'Sat 2026-09-26 13:15 +1245', kind: 'spring-forward' }, - { zone: 'America/Santiago', instant: '2026-09-06T00:30:00Z', offset: 1, local: 'Sat 2026-09-05 20:30 -04', kind: 'spring-forward' }, -]; - -/** The fence zones below that do NOT observe DST -- named here so the module-load warm-up covers them too. */ -const NON_DST_FENCE_ZONES = ['UTC', 'Asia/Shanghai', 'Asia/Kolkata', 'Australia/Perth']; - -warmZoneData([...new Set(DST_CELLS.map((c) => c.zone)), ...NON_DST_FENCE_ZONES]); - -const label = (c: Cell) => `${c.zone} @ ${c.instant} (${c.local}) {TODAY() ${c.offset > 0 ? '+' : '-'} ${Math.abs(c.offset)}}`; - -describe('#14852 {TODAY() +/- n} resolves on one calendar, across DST transitions', () => { - for (const c of DST_CELLS) { - it(`${c.kind}: ${label(c)}`, () => { - const expr = `TODAY() ${c.offset > 0 ? '+' : '-'} ${Math.abs(c.offset)}`; - at(c.zone, c.instant, () => { - const expected = utcDayShift(c.instant, c.offset); - - // CONTROL FIRST — if this passes, the cell is not in a - // transition window and the assertion below would be vacuous. - // (It is the whole reason a TZ=UTC-only test is worthless here.) - expect( - mixedCalendarSpelling(c.instant, c.offset), - `${label(c)}: the mixed spelling must DISAGREE here, otherwise this cell pins nothing`, - ).not.toBe(expected); - - expect(tpl(expr), label(c)).toBe(expected); - }); - }); - } - - it('every red cell is live — the control disagrees in all of them', () => { - const live = DST_CELLS.filter((c) => - at(c.zone, c.instant, () => mixedCalendarSpelling(c.instant, c.offset) !== utcDayShift(c.instant, c.offset)), - ); - expect(live.length, 'a cell that no longer flips has stopped guarding the fix').toBe(DST_CELLS.length); - }); - - it('both directions are represented — a day EARLY and a day LATE', () => { - const dirs = new Set( - DST_CELLS.map((c) => - at(c.zone, c.instant, () => - mixedCalendarSpelling(c.instant, c.offset) < utcDayShift(c.instant, c.offset) ? 'early' : 'late', - ), - ), - ); - expect([...dirs].sort()).toEqual(['early', 'late']); - }); -}); - -describe('#14852 the offset branch serves NOW() too — same mutation, same calendar', () => { - for (const c of DST_CELLS.filter((x) => x.offset === 1).slice(0, 4)) { - it(`${c.zone} @ ${c.instant}: {NOW() + 1} is exactly +24h`, () => { - at(c.zone, c.instant, () => { - expect(tpl('NOW() + 1')).toBe(new Date(Date.parse(c.instant) + 86_400_000).toISOString()); - }); - }); - } -}); - -describe('#14852 the offset may come from a variable — same branch', () => { - it('{TODAY() + days} with days=1 lands on the UTC day too', () => { - const c = DST_CELLS[0]; - at(c.zone, c.instant, () => { - expect(tpl('TODAY() + days', { days: 1 })).toBe(utcDayShift(c.instant, 1)); - }); - }); -}); - -// ── Fences: what this change must NOT have moved ────────────────────────── - -describe('#14852 fences — the bare forms and the non-DST zones are untouched', () => { - const ALL_ZONES = [...new Set(DST_CELLS.map((c) => c.zone))]; - - it('bare {TODAY()} still resolves to the UTC day in every zone, including inside a transition window', () => { - for (const c of DST_CELLS) { - at(c.zone, c.instant, () => { - expect(tpl('TODAY()'), `${c.zone} @ ${c.instant}`).toBe(c.instant.slice(0, 10)); - }); - } - }); - - it('bare {NOW()} still renders the instant itself', () => { - for (const zone of ALL_ZONES) { - at(zone, '2026-03-08T00:30:00Z', () => { - expect(tpl('NOW()'), zone).toBe('2026-03-08T00:30:00.000Z'); - }); - } - }); - - it('zones that do not observe DST are unaffected — both spellings already agreed there', () => { - for (const zone of NON_DST_FENCE_ZONES) { - for (const instant of ['2026-03-08T00:30:00Z', '2026-10-31T23:30:00Z', '2026-06-15T12:00:00Z']) { - at(zone, instant, () => { - expect(mixedCalendarSpelling(instant, 1), `${zone} @ ${instant}`).toBe(utcDayShift(instant, 1)); - expect(tpl('TODAY() + 1'), `${zone} @ ${instant}`).toBe(utcDayShift(instant, 1)); - }); - } - } - }); - - it('an ordinary instant in a DST zone is unaffected — the local day is 24h there', () => { - for (const zone of ALL_ZONES) { - at(zone, '2026-06-15T12:00:00Z', () => { - expect(mixedCalendarSpelling('2026-06-15T12:00:00Z', 1), zone).toBe('2026-06-16'); - expect(tpl('TODAY() + 1'), zone).toBe('2026-06-16'); - }); - } - }); - - it('a large offset still lands on the UTC day (the consumers use +90 and +120)', () => { - const c = DST_CELLS[0]; - at(c.zone, c.instant, () => { - expect(tpl('TODAY() + 90')).toBe(utcDayShift(c.instant, 90)); - expect(tpl('TODAY() + 120')).toBe(utcDayShift(c.instant, 120)); - }); - }); - - it('the process timezone is restored after every case', () => { - expect(process.env.TZ).toBe(REAL_TZ); - }); -}); diff --git a/packages/services/service-automation/src/builtin/template-functions.test.ts b/packages/services/service-automation/src/builtin/template-functions.test.ts deleted file mode 100644 index 3e925956a6b..00000000000 --- a/packages/services/service-automation/src/builtin/template-functions.test.ts +++ /dev/null @@ -1,228 +0,0 @@ -// Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license. - -/** - * Commit 815585513 — the flow VALUE-expression function table, both halves of the ruling: - * - * 1. `round` / `floor` / `ceil` / `abs` / `min` / `max` work in value - * expressions, every name and semantic mirrored **1:1 from the CEL - * stdlib** — proven here by PARITY, not by re-stating the numbers: one - * input grid drives BOTH engines (`ExpressionEngine` CEL vs the template - * evaluator) and the results must agree exactly. A drift in either - * implementation reds this file. - * - * 2. The silent-`null` rewrite is gone for CALL positions: an unknown - * function name is a LOUD named error ({@link FlowExpressionFunctionError}, - * guard-marked so a `fault` edge cannot swallow it) — never an `undefined` - * field write. This is the half the ruling weights most: without it the - * seventh name anyone types silently falls into the same trap the six - * functions climbed out of. - * - * Over-denial controls pin that the diagnostic did NOT become a blanket - * refusal: operator-only arithmetic, `NOW()`/`TODAY()` (whole-token macros), - * and unresolved NON-call identifiers all behave exactly as before. - */ - -import { describe, it, expect } from 'vitest'; -import { ExpressionEngine } from '@objectstack/formula'; -import { interpolateString, FlowExpressionFunctionError } from './template.js'; -import { isGuardRefusal } from '../guard-refusal.js'; - -const ctx = {} as any; - -function tpl(expr: string, vars: Record = {}): unknown { - return interpolateString(`{${expr}}`, new Map(Object.entries(vars)), ctx); -} - -function cel(source: string): { ok: boolean; value?: unknown; error?: { message: string } } { - // The CEL engine requires a context object (it reads `ctx.timezone`); - // an empty one gives the stdlib its UTC defaults. - return ExpressionEngine.evaluate({ dialect: 'cel', source } as any, {}) as any; -} - -// ── Half 1: the six functions, semantics pinned BY PARITY with CEL ───────── - -describe('value-expression functions mirror the CEL stdlib 1:1 (#11060)', () => { - // The grid deliberately covers the rounding-mode traps: halves (JS - // Math.round rounds half toward +∞: round(-1.5) === -1, round(2.5) === 3), - // negative floor/ceil direction (toward −∞ / +∞), zero, and the issue's - // own unrounded product ×100. - const UNARY_GRID = [1.4, 1.5, 1.6, 2.5, -1.2, -1.5, -2.5, 0, 0.5, -0.5, 3.7, -3.7, 12599999.999999998, 125999.99999999999]; - - for (const fn of ['round', 'floor', 'ceil', 'abs'] as const) { - it(`${fn}(x) agrees with ExpressionEngine CEL on every grid input`, () => { - for (const x of UNARY_GRID) { - const c = cel(`${fn}(${x})`); - expect(c.ok, `CEL ${fn}(${x}) must evaluate: ${JSON.stringify(c)}`).toBe(true); - const t = tpl(`${fn}(x)`, { x }); - // CEL int results come back as plain numbers (safe range) — - // Object.is equality, so a -0/0 or carrier drift also reds. - expect(t, `${fn}(${x}): template=${String(t)} cel=${String(c.value)}`).toBe(c.value); - } - }); - } - - it('min/max agree with CEL and return the operand verbatim (type preserved)', () => { - const PAIRS: Array<[unknown, unknown]> = [[1, 2], [2, 1], [1.5, 1.5], [-1, -2], [0.1, 0.2], [180000, 200000]]; - for (const fn of ['min', 'max'] as const) { - for (const [a, b] of PAIRS) { - const c = cel(`${fn}(${String(a)}, ${String(b)})`); - expect(c.ok, `CEL ${fn}(${String(a)}, ${String(b)})`).toBe(true); - expect(tpl(`${fn}(a, b)`, { a, b })).toBe(c.value); - } - } - // Operand-verbatim on non-numeric operands (numeric comparison is NaN - // → both predicates false → second operand), same lambda both sides. - const cs = cel(`min("apple", "banana")`); - expect(cs.ok).toBe(true); - expect(tpl(`min(a, b)`, { a: 'apple', b: 'banana' })).toBe(cs.value); - }); - - it("abs of a non-numeric value is NaN in BOTH engines (CEL's abs(dyn): double does not fault)", () => { - const c = cel(`abs("x")`); - expect(c.ok).toBe(true); - expect(Number.isNaN(c.value as number), `cel abs("x") = ${String(c.value)}`).toBe(true); - const t = tpl(`abs(v)`, { v: 'x' }); - expect(Number.isNaN(t as number), `template abs("x") = ${String(t)}`).toBe(true); - }); - - it('round of a non-numeric value REFUSES in both engines (CEL faults on BigInt(NaN); here it is the named error)', () => { - // CEL side: stdlib round → BigInt(Math.round(Number("x"))) throws. - const c = cel(`round("x")`); - expect(c.ok, 'CEL must fault, not return a value').toBe(false); - // Template side: same direction, as the named diagnostic. - let caught: unknown; - try { tpl(`round(v)`, { v: 'x' }); } catch (e) { caught = e; } - expect(caught).toBeInstanceOf(FlowExpressionFunctionError); - expect((caught as FlowExpressionFunctionError).problem).toBe('argument'); - expect((caught as FlowExpressionFunctionError).fn).toBe('round'); - }); - - it('wrong arity REFUSES in both engines — round(x, 2) has NO precision form, and the refusal says what to write', () => { - const c = cel('round(1.5, 2)'); - expect(c.ok, 'CEL has no round/2 overload').toBe(false); - let caught: unknown; - try { tpl('round(x, 2)', { x: 1.5 }); } catch (e) { caught = e; } - expect(caught).toBeInstanceOf(FlowExpressionFunctionError); - const err = caught as FlowExpressionFunctionError; - expect(err.problem).toBe('arity'); - expect(err.fn).toBe('round'); - // The prescription IS the contract here: the refusal must hand the - // author the N-decimal pattern that is right in BOTH dialects — - // `/ 100.0`, because in CEL `round()` is an int and `int / int` is - // integer division, so the `/ 100` spelling drops the decimals the - // moment it is copied into a CEL slot (#11182 ruling D point 2). - expect(err.message).toContain('round(x * 100) / 100.0'); - const [c100, c1000] = [cel('round(1234.5678 * 100.0) / 100'), cel('round(1234.5678 * 100.0) / 100.0')]; - expect(c100.ok && c1000.ok).toBe(true); - expect([c100.ok && c100.value, c1000.ok && c1000.value], 'the reason the decimal point is prescribed').toEqual([1234, 1234.57]); - expect(tpl('round(x * 100) / 100.0', { x: 1234.5678 }), 'and the template dialect agrees with it').toBe(1234.57); - // min/max are exactly binary, like their CEL registrations. - expect(() => tpl('min(1)', {})).toThrow(FlowExpressionFunctionError); - expect(() => tpl('min(1, 2, 3)', {})).toThrow(FlowExpressionFunctionError); - }); - - it('declared divergence pin: beyond MAX_SAFE_INTEGER, CEL switches carrier to string; the template dialect refuses loudly', () => { - const c = cel('round(10000000000000000.0)'); // 1e16 > 2^53 - expect(c.ok).toBe(true); - expect(typeof c.value, 'CEL boundary hands back a string carrier here').toBe('string'); - // A string riding into this dialect's JS arithmetic would corrupt - // silently (`"1e16" / 100`), so the mirror's documented edge is a - // named error instead. - let caught: unknown; - try { tpl('round(x)', { x: 1e16 }); } catch (e) { caught = e; } - expect(caught).toBeInstanceOf(FlowExpressionFunctionError); - expect((caught as FlowExpressionFunctionError).problem).toBe('argument'); - }); - - it('the issue’s oracle shape: round(amount * (1 - discount / 100) * 100) / 100 lands the scale-2 value', () => { - // 180000 * (1 - 30/100) = 125999.99999999999 — the raw product #7501 - // refuses on a scale: 2 field. The CEL-identical authoring pattern - // produces the exact representable value. - expect(tpl('amount * (1 - discount / 100)', { amount: 180000, discount: 30 })).toBe(125999.99999999999); - expect(tpl('round(amount * (1 - discount / 100) * 100) / 100', { amount: 180000, discount: 30 })).toBe(126000); - // A case where the cents matter (integer round would be WRONG): - expect(tpl('round(amount * (1 - discount / 100) * 100) / 100', { amount: 199.99, discount: 15 })).toBe(169.99); - }); - - it('functions compose with variables, nesting, and embedded substitution', () => { - expect(tpl('min(round(a), ceil(b))', { a: 2.6, b: 1.2 })).toBe(2); - expect(tpl('max(abs(a), b)', { a: -5, b: 3 })).toBe(5); - expect(interpolateString('Total: {round(amount * 1.5)}', new Map([['amount', 180000]]), ctx)).toBe('Total: 270000'); - }); - - it('in call position the function table wins; bare, a variable of the same name still wins', () => { - const vars = { round: 99 }; - expect(tpl('round', vars)).toBe(99); // bare → variable (unchanged) - expect(tpl('round(1.4)', vars)).toBe(1); // call → the function - }); -}); - -// ── Half 2: the LOUD diagnostic — unknown function ⇒ named error, never null ─ - -describe('unknown function in call position is a named, guard-marked error (#11060)', () => { - const CASES: Array<{ expr: string; fn: string; hint?: string }> = [ - // The issue's measured spellings, verbatim: - { expr: 'ROUND(amount * (1 - discount / 100), 2)', fn: 'ROUND', hint: "Did you mean 'round'" }, - { expr: 'round2(x)', fn: 'round2', hint: "Did you mean 'round'" }, - { expr: 'Math.round(amount)', fn: 'Math.round', hint: "Did you mean 'round'" }, - { expr: 'Number(x)', fn: 'Number' }, - { expr: '(amount * 0.7).toFixed(2)', fn: 'toFixed' }, - ]; - - for (const { expr, fn, hint } of CASES) { - it(`'${expr}' names '${fn}' instead of writing undefined`, () => { - let caught: unknown; - try { tpl(expr, { amount: 180000, discount: 30, x: 1 }); } catch (e) { caught = e; } - expect(caught, `'${expr}' must throw — got value instead`).toBeInstanceOf(FlowExpressionFunctionError); - const err = caught as FlowExpressionFunctionError; - expect(err.problem).toBe('unknown-function'); - expect(err.fn).toBe(fn); - expect(err.message).toContain(`'${fn}'`); - if (hint) expect(err.message).toContain(hint); - // #3863 — the metadata is wrong; a `fault` edge must not route it. - expect(isGuardRefusal(err), 'must be guard-marked').toBe(true); - }); - } - - it('the diagnostic reaches EMBEDDED tokens too — no silent empty-string leg left', () => { - expect(() => interpolateString('Total: {ROUND(amount)}', new Map([['amount', 1]]), ctx)) - .toThrow(FlowExpressionFunctionError); - }); - - it('NOW/TODAY misused inside arithmetic get the whole-token guidance (they never worked here — but now they say so)', () => { - let caught: unknown; - try { tpl('TODAY() + amount * 2', { amount: 1 }); } catch (e) { caught = e; } - expect(caught).toBeInstanceOf(FlowExpressionFunctionError); - expect((caught as FlowExpressionFunctionError).message).toContain('whole token'); - }); -}); - -// ── Over-denial controls: everything that worked keeps working ───────────── - -describe('over-denial controls — the diagnostic is not a blanket refusal (#11060)', () => { - it('operator-only arithmetic (no identifiers in call position) still evaluates', () => { - const x = 125999.99999999999; - expect(tpl('(x * 100 + 0.5 - ((x * 100 + 0.5) % 1)) / 100', { x })).toBe(126000); - expect(tpl('a + b * 2', { a: 1, b: 3 })).toBe(7); - }); - - it('NOW()/TODAY() whole-token macros are unchanged', () => { - expect(String(tpl('NOW()'))).toMatch(/^\d{4}-\d{2}-\d{2}T\d{2}:\d{2}:\d{2}/); - expect(String(tpl('TODAY()'))).toMatch(/^\d{4}-\d{2}-\d{2}$/); - // #14852: the ORACLE has to be spelled on one calendar too. With - // `setDate`/`getDate` it re-stated the very two-calendar defect it was - // checking against, so across a DST transition it would have FOLLOWED - // the implementation instead of catching it. Indistinguishable at - // TZ=UTC, which is why it read as correct for as long as it did; the - // transition windows themselves are pinned in - // `template-date-offset-dst.test.ts`. - const plus90 = new Date(); - plus90.setUTCDate(plus90.getUTCDate() + 90); - expect(tpl('TODAY() + 90')).toBe(plus90.toISOString().slice(0, 10)); - }); - - it('unresolved NON-call identifiers keep the documented fail-soft contract', () => { - expect(tpl('missing')).toBeUndefined(); - expect(interpolateString('x={missing}', new Map(), ctx)).toBe('x='); - }); -}); diff --git a/packages/services/service-automation/src/builtin/template-object-token.test.ts b/packages/services/service-automation/src/builtin/template-object-token.test.ts deleted file mode 100644 index 5d302cf5d16..00000000000 --- a/packages/services/service-automation/src/builtin/template-object-token.test.ts +++ /dev/null @@ -1,63 +0,0 @@ -// Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license. - -/** - * #3450 — a flow string template that embeds an OBJECT-valued token (most - * notably the engine's `$error` = `{nodeId, message, ...}`) must never render - * as the useless `[object Object]`. Embedded object/array tokens are JSON- - * serialized so the text stays legible and still carries the message. - */ -import { describe, it, expect } from 'vitest'; -import { interpolateString, stringifyForTemplate } from './template.js'; - -const ctx = {} as any; -const errObj = { nodeId: 'create_contact', message: 'crm_contact is required' }; -const vars = new Map([ - ['$error', errObj], - ['count', 3], - ['flag', true], -]); - -describe('stringifyForTemplate (#3450)', () => { - it('serializes objects and arrays as JSON, never [object Object]', () => { - expect(stringifyForTemplate(errObj)).toBe(JSON.stringify(errObj)); - expect(stringifyForTemplate(errObj)).not.toBe('[object Object]'); - expect(stringifyForTemplate([1, 2])).toBe('[1,2]'); - }); - - it('passes primitives through and renders null/undefined as empty', () => { - expect(stringifyForTemplate('hi')).toBe('hi'); - expect(stringifyForTemplate(3)).toBe('3'); - expect(stringifyForTemplate(true)).toBe('true'); - expect(stringifyForTemplate(null)).toBe(''); - expect(stringifyForTemplate(undefined)).toBe(''); - }); - - it('falls back without throwing on a circular object', () => { - const circular: Record = {}; - circular.self = circular; - expect(() => stringifyForTemplate(circular)).not.toThrow(); - }); -}); - -describe('interpolateString with object tokens (#3450)', () => { - it('renders an embedded $error object as readable JSON, not [object Object]', () => { - const out = interpolateString('Conversion failed: {$error}', vars, ctx) as string; - expect(out).not.toContain('[object Object]'); - expect(out).toContain('crm_contact is required'); - expect(out).toBe(`Conversion failed: ${JSON.stringify(errObj)}`); - }); - - it('still resolves the dotted path to just the message string', () => { - expect(interpolateString('Failed: {$error.message}', vars, ctx)).toBe('Failed: crm_contact is required'); - }); - - it('preserves the raw object for a sole token (type preserved)', () => { - // A single-token template returns the raw value so typed config fields keep - // their type; only EMBEDDED substitution coerces to text. - expect(interpolateString('{$error}', vars, ctx)).toEqual(errObj); - }); - - it('leaves primitive embedded tokens unchanged', () => { - expect(interpolateString('n={count};f={flag}', vars, ctx)).toBe('n=3;f=true'); - }); -}); diff --git a/packages/services/service-automation/src/builtin/template.ts b/packages/services/service-automation/src/builtin/template.ts index c1f54b53fbc..15ae86e126e 100644 --- a/packages/services/service-automation/src/builtin/template.ts +++ b/packages/services/service-automation/src/builtin/template.ts @@ -1,386 +1,33 @@ // Copyright (c) 2025 ObjectStack. Licensed under the Apache-2.0 license. /** - * Template interpolation helpers shared across node executors. + * The flow's TEXT-slot renderer, and the variable map every node executor + * reads. * - * Supported syntax (intentionally minimal — no full expression language): + * A text slot — a notify `title` / `message` / `actionUrl`, an http `url` and + * each `headers` value, a screen `title` / `description`, a refusing `end` + * node's `message` (#22110, ADR-0032 D3; #19939) — renders ADR-0032 §3's + * `{{ }}` holes through the formula template engine, via + * {@link renderTextSlot} below. A single-brace `{…}` token there is refused + * before a run starts (`@objectstack/spec/automation`'s + * `flow-text-slot-template.ts`). * - * {variable} → variables.get('variable') - * {variable.path.segment} → walks dotted path on the resolved value - * {list.0} / {rec.items.2} → numeric segments index into arrays - * {$User.Id} → reads from context.userId - * {$User.Email} → reads from context.user?.email - * {NOW()} → ISO timestamp at evaluation time - * {TODAY()} → YYYY-MM-DD at evaluation time - * {TODAY() + 90} → date + N days (days only, integer) - * {round(x)} {floor(x)} {ceil(x)} - * {abs(x)} {min(a, b)} {max(a, b)} - * → the CEL stdlib's numeric six, names and - * semantics mirrored 1:1 (commit 815585513 — see - * KNOWN_EXPRESSION_FUNCTIONS below) - * - * Anything that fails to resolve becomes the literal `null` value (for - * single-token templates) or the empty string (for embedded substitution), - * matching the behavior of common low-code formula engines — with ONE loud - * exception (commit 815585513): an identifier in CALL position (`name(…)`) that is not a - * supported function throws {@link FlowExpressionFunctionError} instead of - * being rewritten to `null`. Before that diagnostic, `ROUND(…)` / - * `Math.round(…)` / `(x).toFixed(2)` all compiled to `null(…)`, the TypeError - * was swallowed, and the field was silently written `undefined` — so a flow - * 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. 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): - * they render ADR-0032 §3's `{{ }}` holes through the formula template engine, - * via {@link renderTextSlot} below, and a `{…}` token there is refused - * (`flow-text-slot-template.ts`). + * Nothing in a flow reads the single-brace `{…}` template dialect any more. + * The 17.x interpolator that lived here — `interpolate`, `interpolateString`, + * `resolveToken` and its function table — is deleted (#19939, ADR-0032 + * Sequencing 6: "delete the single-brace resolver"): every position that read + * it is a VALUE slot (a CEL value envelope, or a literal handed on as written, + * `value-slot-map.ts`), a TEXT slot (`{{ }}` holes), or a literal read as + * written, and the doors refuse a `{…}` token at each with its replacement. + * The spec's `flow-template-token.ts` keeps the judges' frozen reading of the + * retired grammar, so a refusal can still name each token's replacement. */ -import type { AutomationContext } from '@objectstack/spec/contracts'; -import { nearestName, templateEngine } from '@objectstack/formula'; +import { templateEngine } from '@objectstack/formula'; import { markGuardRefusal } from '../guard-refusal.js'; export type VariableMap = Map; -/** - * A function-shaped defect in a flow VALUE expression (commit 815585513) — an unknown - * name in call position, a supported name called at the wrong arity, or an - * argument outside the function's domain. - * - * This is the LOUD half of the ruling commit 815585513 records: the silent-`null` rewrite of - * unknown identifiers hid every one of these as an `undefined` field write. - * The error is a guard refusal (#3863) — the metadata (the authored - * expression) is wrong, re-running the flow unchanged can never succeed, and - * a `fault` edge must not be able to swallow it back into silence. - */ -export class FlowExpressionFunctionError extends Error { - /** The function name as authored (`'ROUND'`, `'Math.round'`, `'round'`). */ - readonly fn: string; - /** Which contract the call broke. */ - readonly problem: 'unknown-function' | 'arity' | 'argument'; - - constructor(fn: string, problem: 'unknown-function' | 'arity' | 'argument', message: string) { - super(message); - this.name = 'FlowExpressionFunctionError'; - this.fn = fn; - this.problem = problem; - markGuardRefusal(this); - } -} - -/** - * The value-expression function table (commit 815585513) — maintainer ruling 2026-08-23: - * exactly `round` / `floor` / `ceil` / `abs` / `min` / `max`, every name and - * semantic mirrored **1:1 from the CEL stdlib** (`@objectstack/formula` - * `src/stdlib.ts`, "Numbers" block), ⛔ no second semantics invented. A parity - * test (`template-functions.test.ts`) drives BOTH engines over one input grid - * so a divergence cannot land silently. - * - * Carrier note — why these return plain JS numbers while the stdlib returns - * BigInt for `round`/`floor`/`ceil`: cel-js carries CEL `int` as BigInt, and - * the CEL engine's public boundary (`cel-engine.ts` `coerce`) hands callers a - * plain number whenever the value fits the safe-integer range. THIS dialect's - * operators are plain JS, where a BigInt result would throw on the next `/` - * (`round(x * 100) / 100.0` — the scale-2 authoring pattern). So the table - * returns exactly the post-coercion value the public CEL surface yields. - * - * ⚠️ The divisor's decimal point is load-bearing, and NOT in this dialect: here - * `/` is JS division, so `/ 100` and `/ 100.0` give the same value. In CEL, - * `round()` returns an `int` and `int / int` is INTEGER division — measured, - * `round(x * 100) / 100` answers `1234` for `x = 1234.5678` where `/ 100.0` - * answers `1234.57` — so the `/ 100` spelling silently drops the decimals the - * moment it is copied into a CEL slot (a value envelope, a formula field). - * `/ 100.0` is right in both dialects, which is why it is the one prescribed - * (#11182 ruling D point 2). The two edges where that value CANNOT be mirrored into JS - * arithmetic are named errors instead of silent corruption: a non-finite - * argument (CEL faults there too — `BigInt(NaN)` throws inside the stdlib) and - * a result beyond `Number.MAX_SAFE_INTEGER` (CEL's boundary switches carrier - * to string there; a string riding into `/ 100` would corrupt silently). - * - * `NOW()` / `TODAY()` are deliberately NOT in this table: they are whole-token - * date macros with their own `± N days` grammar, handled before this path — - * and in a value slot, where this dialect is retired, CEL's `isoDate(today())` - * / `isoDatetime(now())` write the same text (the parity pin beside - * `crud-fields-value-envelope.test.ts`). - */ -const EXPRESSION_FUNCTION_ARITY: Record = { - round: 1, - floor: 1, - ceil: 1, - abs: 1, - min: 2, - max: 2, -}; - -function requireArity(fn: string, args: unknown[]): void { - const want = EXPRESSION_FUNCTION_ARITY[fn]; - if (args.length !== want) { - // cel-js refuses the same call with "no matching overload" — same - // outcome, message written for self-correction (ADR-0032 §1d). The - // `round(x, 2)` precision form is THE anticipated misuse (commit 815585513), so - // its refusal carries the supported spelling. - const precisionHint = fn === 'round' && args.length === 2 - ? ' There is no precision form — the CEL stdlib\'s round() is integer-only; for N-decimal rounding write round(x * 100) / 100.0 (scale 2). Keep the decimal point: in CEL round() returns an int and int / int is integer division, so / 100 drops the decimals there, while / 100.0 is right in both dialects.' - : ''; - throw new FlowExpressionFunctionError( - fn, - 'arity', - `flow value expression: ${fn}() takes exactly ${want} argument${want === 1 ? '' : 's'}, got ${args.length}.${precisionHint}`, - ); - } -} - -/** Mirror of the CEL boundary for an int-typed result — see the table note. */ -function celIntegerResult(fn: string, result: number): number { - if (!Number.isFinite(result)) { - throw new FlowExpressionFunctionError( - fn, - 'argument', - `flow value expression: ${fn}() needs a numeric argument, got a value that is not a finite number. ` + - `(The CEL stdlib faults on the same input.)`, - ); - } - if (Math.abs(result) > Number.MAX_SAFE_INTEGER) { - throw new FlowExpressionFunctionError( - fn, - 'argument', - `flow value expression: ${fn}() result ${result} exceeds the safe integer range — ` + - `it cannot be represented exactly. (The CEL boundary returns a string here, which this ` + - `dialect's arithmetic cannot compose; refusing loudly instead.)`, - ); - } - // JS Math.round(-0.5) / Math.ceil(-0.2) yield -0; the CEL path's - // BigInt(-0) → Number(0n) collapses it to +0. Mirror that collapse — - // the parity test compares with Object.is and would red on -0. - return Object.is(result, -0) ? 0 : result; -} - -type TemplateExpressionFunction = (...args: unknown[]) => unknown; - -const KNOWN_EXPRESSION_FUNCTIONS: Record = { - // stdlib: 'abs(dyn): double' → Math.abs(Number(x)) — a non-numeric input - // yields NaN (a double), same as CEL; no fault, mirrored exactly. - abs: (...args) => { requireArity('abs', args); return Math.abs(Number(args[0])); }, - // stdlib: 'round(dyn): int' → BigInt(Math.round(Number(x))). JS Math.round - // rounds half toward +∞ (round(-1.5) === -1) — that IS the mirrored mode. - round: (...args) => { requireArity('round', args); return celIntegerResult('round', Math.round(Number(args[0]))); }, - // stdlib: floor/ceil round toward −∞ / +∞ (floor(-1.2) === -2, ceil(-1.2) === -1). - floor: (...args) => { requireArity('floor', args); return celIntegerResult('floor', Math.floor(Number(args[0]))); }, - ceil: (...args) => { requireArity('ceil', args); return celIntegerResult('ceil', Math.ceil(Number(args[0]))); }, - // stdlib: 'min(dyn, dyn): dyn' — returns the smaller/larger OPERAND - // verbatim (type preserved), comparison numeric. Exact lambda copy. - min: (...args) => { requireArity('min', args); return Number(args[0]) <= Number(args[1]) ? args[0] : args[1]; }, - max: (...args) => { requireArity('max', args); return Number(args[0]) >= Number(args[1]) ? args[0] : args[1]; }, -}; - -const KNOWN_EXPRESSION_FUNCTION_NAMES = Object.keys(KNOWN_EXPRESSION_FUNCTIONS); - -/** Compose the unknown-function refusal, with a did-you-mean when one is near. */ -function unknownFunctionError(name: string, expr: string): FlowExpressionFunctionError { - const last = name.includes('.') ? name.slice(name.lastIndexOf('.') + 1) : name; - const lower = last.toLowerCase(); - const suggestion = KNOWN_EXPRESSION_FUNCTION_NAMES.includes(lower) - ? lower - : nearestName(lower, KNOWN_EXPRESSION_FUNCTION_NAMES); - const hint = name === 'NOW' || name === 'TODAY' - ? ` ${name}() is supported only as the whole token, with an optional ± N day offset (e.g. {${name}() + 7}).` - : suggestion - ? ` Did you mean '${suggestion}'?${name.includes('.') || name !== last ? ' Method/namespace call syntax is not supported — write the bare form.' : ''}` - : ''; - return new FlowExpressionFunctionError( - name, - 'unknown-function', - `flow value expression: unknown function '${name}' in '${expr}'. ` + - `Value expressions support round, floor, ceil, abs, min, max (1:1 with the CEL stdlib) ` + - `and the whole-token date macros NOW() / TODAY().${hint} ` + - `An unknown function is refused here rather than evaluated to null, which would write the field as undefined.`, - ); -} - -/** - * Resolve a dotted path against a base value. - * Returns `undefined` for any missing intermediate node. - */ -function resolvePath(base: unknown, path: string[]): unknown { - let cur: unknown = base; - for (const seg of path) { - if (cur == null) return undefined; - if (typeof cur !== 'object') return undefined; - cur = (cur as Record)[seg]; - } - return cur; -} - -/** - * Resolve a single template token (without braces) to a value. - * Returns `undefined` if the token cannot be resolved. - */ -function resolveToken(token: string, variables: VariableMap, context: AutomationContext): unknown { - const trimmed = token.trim(); - if (!trimmed) return undefined; - - // Built-in date helpers — `NOW()` / `TODAY()` with optional `+ N` day offset. - // The offset may be a literal integer or any token resolvable from `variables`. - const dateFnMatch = /^(NOW|TODAY)\s*\(\s*\)\s*(?:([+\-])\s*(\S+))?$/.exec(trimmed); - if (dateFnMatch) { - const fn = dateFnMatch[1]; - const sign = dateFnMatch[2] === '-' ? -1 : 1; - const offsetRaw = dateFnMatch[3]; - let offset = 0; - if (offsetRaw) { - const asNum = Number(offsetRaw); - if (!isNaN(asNum)) { - offset = asNum; - } else if (variables.has(offsetRaw)) { - offset = Number(variables.get(offsetRaw)) || 0; - } - } - const now = new Date(); - // ONE calendar, and it is UTC — the same one both returns render on - // two lines below (#14852). The old spelling did the day arithmetic - // on the LOCAL calendar (`getDate`/`setDate`) and rendered on the UTC - // one. That is accidentally equivalent only while the local day is 24 - // hours long: `setDate` preserves wall-clock time, so across a DST - // transition the instant moves 23h (spring-forward) or 25h - // (fall-back) instead of n x 24h, and the rendered date lands a day - // early or a day late for the one UTC hour whose shortfall/overshoot - // crosses midnight. Measured over 34 zones x every 30 minutes of 2026 - // x offsets {+1, -1} (1,191,360 pairs): the mixed spelling flips 190 - // of them across 24 DST-observing zones, this spelling flips none. - // Pinned by `template-date-offset-dst.test.ts`, which cannot go red - // at TZ=UTC -- there the two spellings are indistinguishable. - if (offset) now.setUTCDate(now.getUTCDate() + sign * offset); - if (fn === 'NOW') return now.toISOString(); - return now.toISOString().slice(0, 10); - } - - // $User.* shortcuts - if (trimmed.startsWith('$User.')) { - const path = trimmed.slice('$User.'.length).split('.'); - if (path[0] === 'Id') return context.userId; - if (path[0] === 'Email') return resolvePath((context as any).user, ['email', ...path.slice(1)]) ?? undefined; - return resolvePath((context as any).user, path); - } - - // Direct variable / dotted path lookup (fast path, no arithmetic). - // Path segments after the head may be identifiers OR array indices (`\d+`), - // so `{list.0}` / `{record.target_channels.0}` resolve into arrays (#1872). - if (/^[A-Za-z_$][\w$]*(?:\.(?:[A-Za-z_$][\w$]*|\d+))*$/.test(trimmed)) { - const segments = trimmed.split('.'); - const head = segments[0]; - if (variables.has(head)) { - return resolvePath(variables.get(head), segments.slice(1)); - } - if (variables.has(trimmed)) return variables.get(trimmed); - return undefined; - } - - // Arithmetic / mixed expression: substitute variable references (foo, foo.bar) - // with their numeric/string literal forms, then evaluate via Function(). - // Restricted to a safe character set (digits, basic operators, parentheses, - // dots and identifier characters) — never executed on raw user input. - if (!/^[\w\s+\-*/%().,?:<>=!&|"'$]+$/.test(trimmed)) return undefined; - let safe = trimmed; - safe = safe.replace(/([A-Za-z_$][\w$]*(?:\.[A-Za-z_$][\w$]*)*)/g, (match, _p1, offset: number, whole: string) => { - // Don't substitute reserved literals - if (match === 'true' || match === 'false' || match === 'null' || match === 'undefined') return match; - // CALL position (`name(…)`) resolves against the function table, never - // against flow variables (commit 815585513). A known name stays literal — it is - // bound as a Function parameter below. An unknown one is the loud half - // of the ruling: refuse with a named error instead of the old `null` - // rewrite, whose swallowed TypeError wrote the field as `undefined`. - // (The lookahead reads the ORIGINAL string — String.replace never - // rescans substituted output, so a variable's value cannot fabricate a - // call position.) - if (/^\s*\(/.test(whole.slice(offset + match.length))) { - if (Object.prototype.hasOwnProperty.call(KNOWN_EXPRESSION_FUNCTIONS, match)) return match; - throw unknownFunctionError(match, trimmed); - } - const segs = match.split('.'); - const head = segs[0]; - let val: unknown; - if (variables.has(head)) val = resolvePath(variables.get(head), segs.slice(1)); - else if (variables.has(match)) val = variables.get(match); - if (val === undefined || val === null) return 'null'; - if (typeof val === 'number' || typeof val === 'boolean') return String(val); - return JSON.stringify(String(val)); - }); - try { - const fn = new Function(...KNOWN_EXPRESSION_FUNCTION_NAMES, `"use strict"; return (${safe});`); - return fn(...KNOWN_EXPRESSION_FUNCTION_NAMES.map((n) => KNOWN_EXPRESSION_FUNCTIONS[n])); - } catch (err) { - // The named diagnostics (arity / domain, thrown inside a table - // function) must escape — swallowing them here would re-create the - // exact silence commit 815585513 removed. Everything else (junk syntax after - // substitution) keeps the documented fail-soft contract. - if (err instanceof FlowExpressionFunctionError) throw err; - return undefined; - } -} - -/** - * Coerce a resolved token value to its string form for EMBEDDED substitution — - * a token inside a larger string, where the result is definitionally text. - * - * A bare `String(value)` renders an object/array as the useless `[object Object]` - * / comma-joined form. That is the #3450 trap: a fault handler whose message - * embeds the engine-set `$error` object (`{nodeId, message, ...}`) surfaced as - * `[object Object]` instead of a readable error. Objects/arrays are JSON- - * serialized so the text stays legible (and still carries the message); an - * author who wants only the message uses the dotted path (`{$error.message}`). - * `null`/`undefined` render as '' (an unresolved token contributes nothing). - */ -export function stringifyForTemplate(value: unknown): string { - if (value === null || value === undefined) return ''; - if (typeof value === 'object') { - try { - return JSON.stringify(value); - } catch { - // Circular / non-serializable — fall back rather than throw mid-flow. - return String(value); - } - } - return String(value); -} - -/** - * Replace `{...}` tokens in a string with resolved values. - * - When the entire string is a single token, returns the raw value (preserving type). - * - Otherwise concatenates string substitutions, with `null`/`undefined` rendered as '' - * and objects/arrays JSON-serialized (never `[object Object]`, #3450). - */ -export function interpolateString( - input: string, - variables: VariableMap, - context: AutomationContext, -): unknown { - if (!input.includes('{')) return input; - const single = /^\{([^{}]+)\}$/.exec(input); - if (single) { - const value = resolveToken(single[1], variables, context); - return value; - } - return input.replace(/\{([^{}]+)\}/g, (_match, expr) => - stringifyForTemplate(resolveToken(expr, variables, context)), - ); -} - /** * A text slot's `{{ }}` template that does not compile (#22110) — a hole * holding logic or an unknown formatter (`{{ a + b }}`, `{{ x | bogus }}`), or @@ -408,9 +55,9 @@ export class FlowTextTemplateError extends Error { * and a node output stored under a flat dotted key (`n1.result`) reachable as * the path it spells (`{{ n1.result }}`). * - * The nesting follows the interpolator's precedence: a declared variable wins - * over a flat key sharing its head (`{a.b}` walked `a`'s value and never read a - * flat `a.b`), so such a key is not merged into the variable. And it builds + * The nesting follows the 17.x interpolator's precedence: a declared variable + * wins over a flat key sharing its head (`{a.b}` walked `a`'s value and never + * read a flat `a.b`), so such a key is not merged into the variable. And it builds * fresh containers only — a variable's own object is never written into. */ export function textTemplateScope(variables: VariableMap): Record { @@ -480,28 +127,3 @@ export function renderTextSlot(value: unknown, variables: VariableMap): string | const text = String(result.value); return text === '' ? undefined : text; } - -/** - * Recursively interpolate template tokens in arbitrary JSON-like values. - */ -export function interpolate( - value: T, - variables: VariableMap, - context: AutomationContext, -): T { - if (typeof value === 'string') { - return interpolateString(value, variables, context) as unknown as T; - } - if (Array.isArray(value)) { - return value.map(v => interpolate(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] = interpolate(v, variables, context); - } - return out as unknown as T; - } - return value; -} - diff --git a/packages/services/service-automation/src/builtin/text-slot-template.test.ts b/packages/services/service-automation/src/builtin/text-slot-template.test.ts index 20e12f1f191..d18ce8afcb8 100644 --- a/packages/services/service-automation/src/builtin/text-slot-template.test.ts +++ b/packages/services/service-automation/src/builtin/text-slot-template.test.ts @@ -31,9 +31,7 @@ import { installBuiltinNodes } from './index.js'; import type { MessagingServiceSurface } from './notify-node.js'; import { FlowTextTemplateError, - interpolateString, renderTextSlot, - stringifyForTemplate, textTemplateScope, type VariableMap, } from './template.js'; @@ -236,19 +234,20 @@ describe('#22110 — renderTextSlot, the one text renderer', () => { }); it('measured against the 17.x interpolator: SAME on a string, number, boolean, null and an ISO date string, DIFF on a Date and a whole-slot object', () => { - const old = (source: string, variables: VariableMap) => - stringifyForTemplate(interpolateString(source, variables, {} as AutomationContext)); - for (const value of ['Acme', 1234.5, 0, true, null, '2026-10-08']) { - const variables = vars({ x: value }); - expect(renderTextSlot('v={{ x }}', variables), String(value)).toBe(old('v={x}', variables)); + // What the 17.x interpolator wrote for `v={x}`, measured while it lived + // (it is deleted, #19939 S7). + for (const [value, old] of [ + ['Acme', 'v=Acme'], [1234.5, 'v=1234.5'], [0, 'v=0'], [true, 'v=true'], [null, 'v='], ['2026-10-08', 'v=2026-10-08'], + ] as const) { + expect(renderTextSlot('v={{ x }}', vars({ x: value })), String(value)).toBe(old); } // The two inputs that make the path spelling lossy (ADR-0087 D2), so no - // conversion rewrites `{x}` to `{{ x }}`: a Date rendered JSON-quoted… + // conversion rewrites `{x}` to `{{ x }}`: a Date it rendered JSON-quoted + // (`v="2026-10-08T09:30:00.000Z"`)… const date = new Date('2026-10-08T09:30:00.000Z'); - expect(old('v={x}', vars({ x: date }))).toBe('v="2026-10-08T09:30:00.000Z"'); expect(renderTextSlot('v={{ x }}', vars({ x: date }))).toBe('v=2026-10-08T09:30:00.000Z'); - // …and a screen / end text that was one token holding an object rendered `String(value)`. - expect(String(interpolateString('{x}', vars({ x: { a: 1 } }), {} as AutomationContext))).toBe('[object Object]'); + // …and a screen / end text that was one token holding an object, which + // it rendered `String(value)` (`[object Object]`). expect(renderTextSlot('{{ x }}', vars({ x: { a: 1 } }))).toBe('{"a":1}'); }); diff --git a/packages/services/service-automation/src/builtin/value-slot-map.ts b/packages/services/service-automation/src/builtin/value-slot-map.ts index 5523329b963..6fbda213ee5 100644 --- a/packages/services/service-automation/src/builtin/value-slot-map.ts +++ b/packages/services/service-automation/src/builtin/value-slot-map.ts @@ -3,7 +3,7 @@ import { isExpressionEnvelopeShaped } from '@objectstack/spec/automation'; import type { AutomationContext } from '@objectstack/spec/contracts'; import type { AutomationEngine } from '../engine.js'; -import { interpolate, type VariableMap } from './template.js'; +import type { VariableMap } from './template.js'; /** * Resolve a value-slot MAP — `{ : }`, every value a `value`-role @@ -43,17 +43,14 @@ import { interpolate, type VariableMap } from './template.js'; * is handed on. The result is the raw value, its type kept: a list stays a * list, a record a record. * - every other value is a literal — a string, an array, a plain object, an - * envelope-shaped object NESTED inside either — and is handed on as it is. - * A `{token}` of the retired template dialect never reaches this point: - * each executor's `parseNodeConfig` refuses it through - * `FlowValueSlotSchema` (the same judge `registerFlow` and - * `objectstack validate` call), so a literal here carries no token, and - * `interpolate()` is the identity on its text. The call is the one the - * CRUD resolver this generalises already made — it hands on a copy, so a - * callee that writes into its input never writes into the node's config — - * and it goes with the single-brace resolver's other residual calls - * (ADR-0032 Sequencing 6). ⛔ Not a fallback for an envelope: the dispatch - * is by shape, and an envelope never reaches the interpolator. + * envelope-shaped object NESTED inside either — and is handed on as it is, + * as a copy ({@link copyLiteral}), so a callee that writes into its input + * never writes into the node's config. A `{token}` of the retired template + * dialect never reaches this point: each executor's `parseNodeConfig` + * refuses it through `FlowValueSlotSchema` (the same judge `registerFlow` + * and `objectstack validate` call), and nothing reads one — the + * single-brace resolver is deleted (ADR-0032 Sequencing 6). ⛔ Not a + * fallback for an envelope: the dispatch is by shape. * * `context` is the run's: it is what `current_user` is in the CEL scope. * `slot` names the map in a refusal (`fields.total`, `input.ownerId`). @@ -89,7 +86,7 @@ export function resolveValueSlotMap( * the dispatch by shape is spelled once: an envelope-shaped value is evaluated * by `AutomationEngine.evaluateValueEnvelope` (`where` names it in a refusal * or a fault, `fields[0].defaultValue`), and every other value is a literal, - * handed on as a copy. + * handed on as a copy ({@link copyLiteral}). */ export function resolveValueSlot( engine: AutomationEngine, @@ -100,7 +97,32 @@ export function resolveValueSlot( ): unknown { return isExpressionEnvelopeShaped(value) ? engine.evaluateValueEnvelope(value, variables, where, context) - : interpolate(value, variables, context); + : copyLiteral(value); +} + +/** + * A literal value-slot value as the node hands it on: arrays and plain + * objects copied at every depth, every other value — a string, a number, a + * `Date`, a class instance — as it is. The copy keeps a callee that writes + * into its input (a child flow's variable, a function's `input`) from writing + * into the node's config, the copy the single-brace interpolator made while + * it walked the value. + * + * Only PLAIN objects are walked (`Object.prototype` or `null` prototype). The + * interpolator walked every object's own entries, so a `Date` written in a + * code-defined flow (`fields: { due: new Date(…) }`) reached the write as + * `{}`; it is handed on as the `Date` it is. + */ +export function copyLiteral(value: unknown): unknown { + if (Array.isArray(value)) return value.map(copyLiteral); + if (value !== null && typeof value === 'object') { + const proto = Object.getPrototypeOf(value); + if (proto !== Object.prototype && proto !== null) return value; + const out: Record = {}; + for (const [key, entry] of Object.entries(value)) out[key] = copyLiteral(entry); + return out; + } + return value; } /** @@ -112,9 +134,8 @@ export function resolveValueSlot( * `AutomationEngine.evaluateValueEnvelope` in the run's scope and context, * like every value slot's — a malformed one, or one that faults on the live * variables (an absent variable, key or index), throws with its source; - * - an inline array is the list AS WRITTEN — never handed to the - * interpolator, whose recursion would resolve a `{…}` in a string the - * executors never interpolated: an inline list's elements are data. + * - an inline array is the list AS WRITTEN: an inline list's elements are + * data, and the executors never read a `{…}` in one. * * Nothing else arrives: each executor's `parseNodeConfig` has refused a string * — a `{…}` token of the retired dialect, and a variable's bare name, which the diff --git a/packages/services/service-automation/src/builtin/value-slot-template-grammar.test.ts b/packages/services/service-automation/src/builtin/value-slot-template-grammar.test.ts index 9331a7257c7..49f94ef5eb7 100644 --- a/packages/services/service-automation/src/builtin/value-slot-template-grammar.test.ts +++ b/packages/services/service-automation/src/builtin/value-slot-template-grammar.test.ts @@ -1,50 +1,43 @@ // Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license. /** - * [#19939] The spec's value-slot template judge reads the `{…}` dialect the - * way THIS package's interpolator does — the drift pin its module docblock - * names (`@objectstack/spec/automation`, `flow-value-slot-template.ts`). + * [#19939] The spec's value-slot template judge, read against what each + * refused `{…}` spelling resolved to under the 17.x interpolator — the values + * pinned here, which that interpolator answered on these variables while it + * lived — and against what its printed remedy evaluates to through THIS + * package's CEL evaluator. * - * The spec cannot import the interpolator, so it carries a copy of - * `resolveToken`'s dispatch: date macro, then `$User.`, then a variable path, - * then an expression. If the two readings drifted, the judge would print the - * wrong remedy for a spelling (a date macro read as a path, a path read as an - * expression), or pass one it should refuse (the dialect surviving in a value - * slot). So the interpolator itself is driven over the same tokens the judge - * classifies: + * The interpolator is deleted (ADR-0032 Sequencing 6). The spec keeps the + * judges' frozen reading of its grammar (`@objectstack/spec/automation`'s + * `flow-template-token.ts`): date macro, then `$User.`, then a variable path, + * then an expression. That reading decides the remedy a refusal prints, so it + * is pinned here by what each remedy evaluates to: * - * - every date macro (refused since #19939 pass 3) resolves through - * `interpolateString` to ISO text, and the judge refuses each with its CEL - * string form (`isoDate(…)` / `isoDatetime(…)`) — read as a date macro, - * never as a path or an expression (that each form writes the same bytes - * over the same instants is the parity pin in - * `crud-fields-value-envelope.test.ts`); - * - the run user (`$User`, refused since #19939 pass 2) resolves through the - * interpolator to the run's `userId` for `$User.Id` and to nothing for every - * other path in a shipped run context, and the remedies the judge prints - * evaluate through the CEL scope's `current_user` to the same answer — with - * a user, and, guarded, without one; - * - every REFUSED spelling resolves through the variables (a path), computes - * (an expression), or resolves to nothing — and the judge refuses each, - * including the dispatch-order edges (`{$User}` with no path, `{NOW}` with - * no call, a padded `{ amount }`); - * - for a refused PATH, the remedy the judge prints reads the same value - * through this package's CEL evaluator (`evaluateValueEnvelope`: the - * author-time envelope check, then the built `@objectstack/formula` engine - * over the flow's real CEL scope) as the interpolator read from the - * template — including a head variable named like an identifier CEL claims - * for itself (`{list.0}` → `vars["list"][0]`, #22290), or one the flow - * scope binds over it (`{vars.0}` → `vars["vars"][0]`), and inside an - * EXPRESSION token (`{int * 2}` → `vars["int"] * 2`) — and the `has()` - * guard it prints answers `null`, rather than failing the run, where the - * variable itself was never bound (#19939 pass 3). + * - every date macro is refused with its CEL string form (`isoDate(…)` / + * `isoDatetime(…)`) — read as a date macro, never as a path or an + * expression; + * - the run user (`$User`) is refused, and the remedies the judge prints + * evaluate through the CEL scope's `current_user` to the run's `userId` — + * with a user, and, guarded, to `null` without one; + * - every REFUSED spelling — a path, an expression, a path that resolved to + * nothing — is refused, including the dispatch-order edges (`{$User}` with + * no path, `{NOW}` with no call, a padded `{ amount }`); + * - for a refused PATH, the remedy the judge prints reads, through this + * package's CEL evaluator (`evaluateValueEnvelope`: the author-time + * envelope check, then the built `@objectstack/formula` engine over the + * flow's real CEL scope), the value the template read — including a head + * variable named like an identifier CEL claims for itself (`{list.0}` → + * `vars["list"][0]`, #22290), or one the flow scope binds over it + * (`{vars.0}` → `vars["vars"][0]`), and inside an EXPRESSION token + * (`{int * 2}` → `vars["int"] * 2`) — and the `has()` guard it prints + * answers `null`, rather than failing the run, where the variable itself + * was never bound (#19939 pass 3). */ import { describe, expect, it } from 'vitest'; import { VALUE_SLOT_TEMPLATE_REFUSAL, valueSlotTemplateRefusals } from '@objectstack/spec/automation'; import { AutomationEngine } from '../engine.js'; import { registerLogicNodes } from './logic-nodes.js'; -import { interpolateString } from './template.js'; const VARIABLES = new Map([ ['amount', 1234.5], @@ -53,9 +46,10 @@ const VARIABLES = new Map([ ['$error', { message: 'boom' }], ['days', 3], ]); -const CONTEXT = { userId: 'usr_1', user: { email: 'ada@example.com' } } as never; -describe('a date macro resolves through the interpolator to ISO text, and is REFUSED as a date macro', () => { +describe('a date macro is REFUSED as a date macro, naming its CEL string form', () => { + const quiet = { info: () => {}, warn: () => {}, error: () => {}, debug: () => {}, child: () => quiet } as never; + const engine = new AutomationEngine(quiet); it.each([ ['{NOW()}', /^\d{4}-\d{2}-\d{2}T/, 'isoDatetime(now())'], ['{TODAY()}', /^\d{4}-\d{2}-\d{2}$/, 'isoDate(today())'], @@ -63,14 +57,15 @@ describe('a date macro resolves through the interpolator to ISO text, and is REF ['{TODAY() - days}', /^\d{4}-\d{2}-\d{2}$/, 'isoDate(addDays(today(), -days))'], ['{NOW() + 2}', /^\d{4}-\d{2}-\d{2}T/, 'isoDatetime(addDays(now(), 2))'], ])('%s — a date macro', (token, shape, source) => { - expect(String(interpolateString(token, VARIABLES, CONTEXT))).toMatch(shape); + // The macro wrote ISO text; its CEL string form writes the same shape. + expect(String(engine.evaluateValueEnvelope({ dialect: 'cel', source }, VARIABLES, 'w'))).toMatch(shape); const refusals = valueSlotTemplateRefusals(token); expect(refusals).toHaveLength(1); expect(refusals[0]!.message).toContain(`Write \`${token}\` as { dialect: 'cel', source: '${source}' }`); }); }); -describe('the run user — `{$User.*}` — is REFUSED, and its remedy reads what the interpolator read', () => { +describe('the run user — `{$User.*}` — is REFUSED, and its remedy reads what the template read', () => { const quiet = { info: () => {}, warn: () => {}, error: () => {}, debug: () => {}, child: () => quiet } as never; const engine = new AutomationEngine(quiet); /** A run context the way the trigger doors build one: no `user` object, ever. */ @@ -86,18 +81,15 @@ describe('the run user — `{$User.*}` — is REFUSED, and its remedy reads what expect(valueSlotTemplateRefusals('{$User.Id}')).toHaveLength(1); const [bare, guarded] = sources('{$User.Id}'); expect([bare, guarded]).toEqual(['current_user.id', 'current_user != null ? current_user.id : null']); - // With a user: all three agree. - expect(interpolateString('{$User.Id}', VARIABLES, SHIPPED)).toBe('usr_1'); + // With a user: both read the run's `userId`, as the template did. expect(engine.evaluateValueEnvelope({ dialect: 'cel', source: bare! }, VARIABLES, 'w', SHIPPED)).toBe('usr_1'); expect(engine.evaluateValueEnvelope({ dialect: 'cel', source: guarded! }, VARIABLES, 'w', SHIPPED)).toBe('usr_1'); - // Without one: the template read nothing, the bare read fails loudly, the guard reads `null`. - expect(interpolateString('{$User.Id}', VARIABLES, USERLESS)).toBeUndefined(); + // Without one (the template read nothing): the bare read fails loudly, the guard reads `null`. expect(() => engine.evaluateValueEnvelope({ dialect: 'cel', source: bare! }, VARIABLES, 'w', USERLESS)).toThrow(/current_user\.id/); expect(engine.evaluateValueEnvelope({ dialect: 'cel', source: guarded! }, VARIABLES, 'w', USERLESS)).toBeNull(); }); it.each(['{$User.Email}', '{$User.Name}', '{$User.id}'])('%s never resolved in a shipped run context — and is refused saying so', (token) => { - expect(interpolateString(token, VARIABLES, SHIPPED)).toBeUndefined(); expect(valueSlotTemplateRefusals(token)[0]!.message).toContain(`\`${token}\` never resolved in any shipped run`); }); @@ -108,24 +100,12 @@ describe('the run user — `{$User.*}` — is REFUSED, and its remedy reads what }); }); -describe('a spelling the retirement REFUSES is one the interpolator reads as template dialect', () => { - it.each([ - ['{amount}', 1234.5], - ['{ amount }', 1234.5], - ['{record.owner}', 'usr_7'], - ['{list.0}', 'a'], - ['{$error.message}', 'boom'], - ])('%s — a variable path, resolved through the variables', (token, value) => { - expect(interpolateString(token, VARIABLES, CONTEXT)).toBe(value); +describe('a spelling of the retired template dialect is REFUSED', () => { + it.each(['{amount}', '{ amount }', '{record.owner}', '{list.0}', '{$error.message}'])('%s — a variable path', (token) => { expect(valueSlotTemplateRefusals(token)).toHaveLength(1); }); - it.each([ - ['{amount * 2}', 2469], - ['{round(amount)}', 1235], - ['{max(amount, 10)}', 1234.5], - ])('%s — an expression, computed', (token, value) => { - expect(interpolateString(token, VARIABLES, CONTEXT)).toBe(value); + it.each(['{amount * 2}', '{round(amount)}', '{max(amount, 10)}'])('%s — an expression', (token) => { expect(valueSlotTemplateRefusals(token)).toHaveLength(1); }); @@ -136,25 +116,21 @@ describe('a spelling the retirement REFUSES is one the interpolator reads as tem // No call: not the date macro — a variable path. '{NOW}', '{missing.key}', - ])('%s — a dispatch-order edge: a path that resolves to nothing', (token) => { - expect(interpolateString(token, VARIABLES, CONTEXT)).toBeUndefined(); + ])('%s — a dispatch-order edge: a path that resolved to nothing', (token) => { expect(valueSlotTemplateRefusals(token)).toHaveLength(1); }); - it('text with holes renders through the interpolator, and is refused as one string', () => { - expect(interpolateString('Owner {record.owner} of {record.name}', VARIABLES, CONTEXT)).toBe('Owner usr_7 of Acme'); + it('text with holes is refused as one string', () => { expect(valueSlotTemplateRefusals('Owner {record.owner} of {record.name}')).toHaveLength(1); }); - it('a token-free string is the same text to both: no substitution, no refusal', () => { - expect(interpolateString('approved', VARIABLES, CONTEXT)).toBe('approved'); - expect(interpolateString('a {} b', VARIABLES, CONTEXT)).toBe('a {} b'); + it('a token-free string is no refusal — `{}` holds no token', () => { expect(valueSlotTemplateRefusals('approved')).toEqual([]); expect(valueSlotTemplateRefusals('a {} b')).toEqual([]); }); }); -describe('the remedy a refused path prints reads, through CEL, the value the interpolator read', () => { +describe('the remedy a refused path prints reads, through CEL, the value the template read', () => { const quiet = { info: () => {}, warn: () => {}, error: () => {}, debug: () => {}, child: () => quiet } as never; const engine = new AutomationEngine(quiet); @@ -169,9 +145,8 @@ describe('the remedy a refused path prints reads, through CEL, the value the int return [envelope![1] ?? (JSON.parse(envelope![2]!) as string), ...(guard ? [guard[1]!] : [])]; } - /** Both readings of `token` over `variables`, and every printed spelling evaluated to the interpolator's value. */ + /** Every printed spelling of `token`, evaluated over `variables`, is the value the template read there. */ function expectReadingsAgree(token: string, variables: Map, value: unknown): void { - expect(interpolateString(token, variables, CONTEXT)).toEqual(value); for (const source of printedSpellings(token)) { expect(engine.evaluateValueEnvelope({ dialect: 'cel', source }, variables, token), source).toEqual(value); } @@ -224,7 +199,6 @@ describe('the remedy a refused path prints reads, through CEL, the value the int expect(evaluate(new Map([[root, null]])), `${guard} — ${root} null`).toBeNull(); if (typeof bound === 'object') expect(evaluate(new Map([[root, {}]])), `${guard} — a key absent`).toBeNull(); const variables = new Map([[root, bound]]); - expect(interpolateString(token, variables, CONTEXT)).toEqual(value); expect(evaluate(variables), `${guard} — bound`).toEqual(value); }); @@ -299,23 +273,20 @@ describe('an EXPRESSION token\'s remedy evaluates — every variable path in it return m![1] ?? (JSON.parse(m![2]!) as string); }; - it('`{int * 2}` — a head CEL claims: `vars["int"] * 2`, the value the interpolator computed', () => { + it('`{int * 2}` — a head CEL claims: `vars["int"] * 2`, the value the template computed', () => { const variables = new Map([['int', 5]]); - expect(interpolateString('{int * 2}', variables, CONTEXT)).toBe(10); expect(remedyOf('{int * 2}')).toBe('vars["int"] * 2'); expect(engine.evaluateValueEnvelope({ dialect: 'cel', source: remedyOf('{int * 2}') }, variables, 'w')).toBe(10); }); - it('`{items.0 * 2}` — an index: `items[0] * 2`, which evaluates (the interpolator computed nothing for it)', () => { + it('`{items.0 * 2}` — an index: `items[0] * 2`, which evaluates (the template computed nothing for it)', () => { const variables = new Map([['items', [3, 4]]]); - expect(interpolateString('{items.0 * 2}', variables, CONTEXT)).toBeUndefined(); expect(remedyOf('{items.0 * 2}')).toBe('items[0] * 2'); expect(engine.evaluateValueEnvelope({ dialect: 'cel', source: remedyOf('{items.0 * 2}') }, variables, 'w')).toBe(6); }); it('a `$`-named head and a divisor in one expression', () => { const variables = new Map([['$error', { code: 7 }]]); - expect(interpolateString('{$error.code / 2}', variables, CONTEXT)).toBe(3.5); expect(remedyOf('{$error.code / 2}')).toBe('vars["$error"].code / 2.0'); expect(engine.evaluateValueEnvelope({ dialect: 'cel', source: remedyOf('{$error.code / 2}') }, variables, 'w')).toBe(3.5); }); diff --git a/packages/services/service-automation/src/engine.test.ts b/packages/services/service-automation/src/engine.test.ts index 7bd396a968f..bf78713e7b0 100644 --- a/packages/services/service-automation/src/engine.test.ts +++ b/packages/services/service-automation/src/engine.test.ts @@ -2165,12 +2165,12 @@ describe('AutomationEngine - Safe Expression Evaluation', () => { const vars = new Map(); vars.set('amount', 500); - expect(engine.evaluateCondition('{amount} > 100', vars)).toBe(true); - expect(engine.evaluateCondition('{amount} < 100', vars)).toBe(false); - expect(engine.evaluateCondition('{amount} == 500', vars)).toBe(true); - expect(engine.evaluateCondition('{amount} >= 500', vars)).toBe(true); - expect(engine.evaluateCondition('{amount} <= 500', vars)).toBe(true); - expect(engine.evaluateCondition('{amount} != 100', vars)).toBe(true); + expect(engine.evaluateCondition('amount > 100', vars)).toBe(true); + expect(engine.evaluateCondition('amount < 100', vars)).toBe(false); + expect(engine.evaluateCondition('amount == 500', vars)).toBe(true); + expect(engine.evaluateCondition('amount >= 500', vars)).toBe(true); + expect(engine.evaluateCondition('amount <= 500', vars)).toBe(true); + expect(engine.evaluateCondition('amount != 100', vars)).toBe(true); }); it('should evaluate boolean literals', () => { @@ -2215,16 +2215,12 @@ describe('AutomationEngine - Safe Expression Evaluation', () => { const vars = new Map(); vars.set('status', 'active'); - expect(engine.evaluateCondition('{status} == active', vars)).toBe(true); - expect(engine.evaluateCondition('{status} != inactive', vars)).toBe(true); - // #4336 — a QUOTED literal on the right compares as its contents. This is - // the spelling the flow docs show for a decision node, and it used to - // compare `active` against `'active'` (quotes included) and be false for - // every value of `status`. - expect(engine.evaluateCondition("{status} == 'active'", vars)).toBe(true); - expect(engine.evaluateCondition('{status} == "active"', vars)).toBe(true); - expect(engine.evaluateCondition("{status} == 'closed'", vars)).toBe(false); - expect(engine.evaluateCondition("{status} != 'closed'", vars)).toBe(true); + // A string literal is quoted, either quote — the spelling the flow docs + // show for a decision node. + expect(engine.evaluateCondition("status == 'active'", vars)).toBe(true); + expect(engine.evaluateCondition('status == "active"', vars)).toBe(true); + expect(engine.evaluateCondition("status == 'closed'", vars)).toBe(false); + expect(engine.evaluateCondition("status != 'closed'", vars)).toBe(true); }); }); @@ -2269,34 +2265,52 @@ describe('AutomationEngine - bare-string conditions evaluate as CEL (#4336)', () expect(engine.evaluateCondition("lead_record.status == 'new'", vars)).toBe(false); }); - it('refuses a brace-wrapped reference that resolves to nothing, naming it', () => { - const vars = new Map([['lead_record', { status: 'converted' }]]); - // Silently false today even though the status IS 'converted'. - expect(() => engine.evaluateCondition("{lead_record.status} == 'converted'", vars)) - .toThrow(/`\{lead_record\.status\}` did not resolve/); - expect(() => engine.evaluateCondition("{lead_record.status} == 'converted'", vars)) - .toThrow(/Drop the braces/); - // Same for a brace-wrapped truthy gate. - expect(() => engine.evaluateCondition('{record.isActive}', new Map([['record', { isActive: true }]]))) - .toThrow(/did not resolve/); - }); - - it('refuses a template-dialect condition it cannot turn into a predicate', () => { - // `{status}` substitutes to `open` — not a boolean, not a number, and no - // operator to compare it with. That used to be `false`. - expect(() => engine.evaluateCondition('{status}', new Map([['status', 'open']]))) - .toThrow(/is not a predicate/); - // A boolean or numeric value still reads as a gate. - expect(engine.evaluateCondition('{flag}', new Map([['flag', true]]))).toBe(true); - expect(engine.evaluateCondition('{flag}', new Map([['flag', false]]))).toBe(false); - expect(engine.evaluateCondition('{count}', new Map([['count', 3]]))).toBe(true); - expect(engine.evaluateCondition('{count}', new Map([['count', 0]]))).toBe(false); + // [#19939 S7] The legacy `{var}` template path is deleted with the + // single-brace resolver (ADR-0032 Sequencing 6). It answered `{flag}`, + // `{count}` and `{amount} > 100` by substituting text; `registerFlow` + // refuses every one of them at every condition slot, so only a direct call + // reached it. A braced condition is now the #1491 brace trap: refused as a + // CEL fault naming its source, whatever the variables hold. + it.each([ + ["{lead_record.status} == 'converted'", new Map([['lead_record', { status: 'converted' }]])], + ['{record.isActive}', new Map([['record', { isActive: true }]])], + ['{status}', new Map([['status', 'open']])], + ['{flag}', new Map([['flag', true]])], + ['{count}', new Map([['count', 3]])], + ['{amount} > 100', new Map([['amount', 500]])], + ])('refuses the retired `{var}` spelling %s as the brace trap, naming its source', (source, vars) => { + let thrown: Error | undefined; + try { + engine.evaluateCondition(source, vars); + } catch (err) { + thrown = err as Error; + } + expect(thrown, source).toBeInstanceOf(Error); + expect(thrown!.message).toContain('condition failed to evaluate as CEL'); + expect(thrown!.message).toContain('do not wrap field references in `{…}` template braces'); + expect(thrown!.message).toContain(`source: \`${source}\``); + }); + + it.each(['template', 'flow'])('refuses a `%s` envelope before evaluating it — braced or not', (dialect) => { + const vars = new Map([['status', 'open']]); + for (const source of ["{status} == 'open'", "status == 'open'"]) { + let thrown: Error | undefined; + try { + engine.evaluateCondition({ dialect, source }, vars); + } catch (err) { + thrown = err as Error; + } + expect(thrown, source).toBeInstanceOf(Error); + expect(thrown!.message).toContain(`the \`${dialect}\` dialect is not evaluated`); + expect(thrown!.message).toContain(`source: \`${source}\``); + } + // Control: the same brace-free source, as CEL, evaluates. + expect(engine.evaluateCondition({ dialect: 'cel', source: "status == 'open'" }, vars)).toBe(true); }); it('does not mistake braces inside a string literal for a template hole', () => { - // `'{pending}'` is text the predicate compares AGAINST, not a reference to - // substitute. The dialect sniff reads the source outside string literals, - // so this stays CEL and compares the field. + // `'{pending}'` is text the predicate compares AGAINST, not a reference: + // a CEL string literal, so the field is compared. expect(engine.evaluateCondition("record.label == '{pending}'", new Map([['record', { label: '{pending}' }]]))).toBe(true); expect(engine.evaluateCondition("record.label == '{pending}'", @@ -2304,9 +2318,8 @@ describe('AutomationEngine - bare-string conditions evaluate as CEL (#4336)', () }); it('keeps braces inside an explicit CEL envelope a hard error', () => { - // The dialect sniff applies only where no dialect was stated. `dialect: - // 'cel'` is the author saying "this is CEL", where `{…}` is a map literal - // and the #1491 brace-trap. + // `dialect: 'cel'` is the author saying "this is CEL", where `{…}` is a + // map literal and the #1491 brace-trap — as it is in bare text. expect(() => engine.evaluateCondition({ dialect: 'cel', source: '{record.rating} >= 4' }, new Map([['record', { rating: 5 }]]))).toThrow(/source:/); }); diff --git a/packages/services/service-automation/src/engine.ts b/packages/services/service-automation/src/engine.ts index 3c5268345d7..47eb2c9bbe2 100644 --- a/packages/services/service-automation/src/engine.ts +++ b/packages/services/service-automation/src/engine.ts @@ -89,61 +89,6 @@ import { ConnectorSchema } from '@objectstack/spec/integration'; // engine at module load in both ESM and CJS builds. import { ExpressionEngine, validateExpression, nearestName } from '@objectstack/formula'; -/** - * A bare **dotted reference** (`record.amount`, `row.shouldRun`, - * `previous.status`) — a CEL path, and a shape `{var}` template substitution can - * never leave behind, since it replaces the whole `{…}` token with a value. - * Each segment must start like an identifier, so numeric literals (`1.5`, - * `500.00`) are not references. See {@link AutomationEngine.refuseUnresolvedCelOperand}. - */ -const UNRESOLVED_CEL_REFERENCE = /^[A-Za-z_$][A-Za-z0-9_$]*(?:\.[A-Za-z_$][A-Za-z0-9_$]*)+$/; - -/** - * A legacy single-brace **template hole** — `{amount}`, `{get_lead.id}`. This is - * the exact shape `{var}` substitution can consume (it splits on the literal - * `{}` text for each variable key), which is what makes it a sound dialect - * discriminator in {@link AutomationEngine.evaluateCondition}: a condition - * containing one was written in the template dialect, and a condition containing - * none was written as CEL (#4336). - * - * No whitespace is tolerated inside the braces, deliberately — `{ amount }` - * is not a token substitution can resolve, so treating it as a hole would only - * move the failure. It is not valid CEL either (a map literal needs `key: value` - * pairs), so it lands on the CEL path and gets the brace-trap diagnostic. - */ -const TEMPLATE_HOLE = /\{[A-Za-z_$][A-Za-z0-9_$]*(?:\.[A-Za-z_$][A-Za-z0-9_$]*)*\}/g; - -/** A quoted string literal, anywhere in a condition. */ -const QUOTED_SEGMENT = /'[^']*'|"[^"]*"/g; - -/** A single-quoted or double-quoted string literal, whole-operand. */ -const QUOTED_LITERAL = /^'([^']*)'$|^"([^"]*)"$/; - -/** - * Every legacy `{var}` hole in `source`, ignoring any that sits **inside a - * string literal** — `record.name == '{unresolved}'` is a CEL predicate - * comparing against text that happens to contain braces, not a template. - * - * This is both the dialect discriminator and the unresolved-hole report, so the - * two can never disagree about what counts as a hole. It reads the AUTHORED - * source rather than the substituted result, so a variable whose *value* - * contains braces cannot masquerade as an unresolved reference. - */ -function templateHoles(source: string): string[] { - return source.replace(QUOTED_SEGMENT, '').match(TEMPLATE_HOLE) ?? []; -} - -/** - * Strip one layer of matching quotes from a template-dialect operand, so a - * quoted string literal compares as its contents (#4336). Anything that is not - * a whole quoted literal is returned untouched — including a value that merely - * contains a quote. - */ -function unquoteLiteral(operand: string): string { - const m = QUOTED_LITERAL.exec(operand); - return m ? (m[1] ?? m[2] ?? '') : operand; -} - /** * The value the flow CEL scope binds as `current_user` for a run (#19939 — * the maintainer's ruling on its second pass, Q1 A and Q2 A): the run's @@ -1548,8 +1493,8 @@ function applyResumeSignal( writes.push([`${nodeId}.${key}`, value]); } // Bare flow variables — a `screen` node's collected inputs land under their - // plain names so downstream `{var}` interpolation / conditions read them - // directly (e.g. `new_assignee` → update_record fields). + // plain names so downstream CEL envelopes, text-slot holes and conditions + // read them directly (e.g. `new_assignee` → update_record fields). for (const [key, value] of Object.entries(signal.variables ?? {})) { writes.push([key, value]); } @@ -10874,8 +10819,9 @@ export class AutomationEngine implements IAutomationService { // [#15137] `value` slots — the ruled `assignment.assignments.*` // and, since #19938, `create_record` / `update_record` // `fields.*`. The resolver emits ONLY envelope-shaped objects - // for this role (a plain string there is `{token}` - // interpolation, every other literal is data), so everything + // for this role (a plain string there is literal text, whose + // `{token}` the value-slot judge above refuses, and every other + // literal is data), so everything // that arrives here is an author saying "this is an // expression" and must be a valid one. Same severity as a // predicate — a malformed envelope stops the flow registering @@ -12096,7 +12042,7 @@ export class AutomationEngine implements IAutomationService { * `id`, `positions`, `organizationId` and the `isPlatformAdmin` flag * derived from them; no run carries an email or a name. This is what the * retired `{$User.Id}` value-slot token is refused in favour of, so it - * reads the same context the interpolator's `$User.Id` branch reads. + * reads the same context the 17.x interpolator's `$User.Id` branch read. * * `vars` and `current_user` are bound AFTER the variables are spread, so * each wins over a flow variable of the same name — which is then read as @@ -12315,38 +12261,32 @@ export class AutomationEngine implements IAutomationService { /** * Evaluate a flow condition to a boolean. * - * ## Which dialect a condition is in + * ## A condition is CEL * - * A condition is **CEL** unless it is written in the legacy single-brace - * `{var}` template dialect — and that is decided by looking at the *source*, - * not at whether an envelope happens to be present (#4336). + * Every condition is **CEL**, wherever it is authored: bare text, an + * envelope with `dialect: 'cel'`, or an envelope naming no dialect. * - * It used to be decided by the envelope, and that was the bug: only an - * `{ dialect, source }` envelope reached the CEL engine, so a condition - * authored as a plain string — the shape `ExpressionInput` accepts by design, - * and the shape every node `config` still holds, since `FlowNodeSchema.config` - * is an open `z.record` no transform can reach — fell through to the template - * path and was compared **as text**. The failure direction depended on the - * predicate, which is what made it dangerous: + * The dialect used to be decided by the envelope, and that was #4336's bug: + * only an `{ dialect, source }` envelope reached the CEL engine, so a + * condition authored as a plain string — the shape `ExpressionInput` + * accepts by design, and the shape every node `config` still holds, since + * `FlowNodeSchema.config` is an open `z.record` no transform can reach — + * fell through to the single-brace `{var}` template path and was compared + * **as text**: * * 'existingTask == null' → 'existingTask' === 'null' → always FALSE * 'record.rating >= 4' → 'record.rating' >= '4' → always TRUE * - * — one gate that never opens, one branch pinned open, both reporting - * `success`. Reading the source instead means the same predicate evaluates - * the same way wherever it is authored: an edge (parsed into an envelope by - * `FlowEdgeSchema`), a start-node gate, or a `decision` node's - * `config.conditions[].expression`, which no schema normalizes. - * - * The `{var}` dialect stays supported for the flows that use it — but it no - * longer answers `false` when it could not resolve something. Per ADR-0032 - * §1c a predicate that cannot be evaluated is a **fault**, never a quiet - * branch decision, so an unresolved hole is refused with the source attached. - * - * Braces inside a **CEL envelope** remain the #1491 brace-trap and still - * throw: an explicit `dialect: 'cel'` is the author saying "this is CEL", and - * `{…}` is a map literal there. The sniff only applies where the dialect was - * never stated. + * #4336 then read the source instead, and kept the template path for a + * string carrying a `{var}` hole. That path is deleted with the + * single-brace resolver (#19939, ADR-0032 Sequencing 6): no registered flow + * reached it — `registerFlow` refuses a braced predicate and a `template` or + * `flow` envelope at every condition slot (ADR-0032 §1a) — so it answered + * only a direct call. A braced string is now the #1491 brace-trap, refused + * as the CEL fault below with the source attached; a `template` or `flow` + * envelope is refused before any evaluation, never re-read as CEL under a + * dialect its author did not write. Per ADR-0032 §1c a predicate that + * cannot be evaluated is a **fault**, never a quiet branch decision. * * ## The shape gate, shared with registration (#16038) * @@ -12402,7 +12342,17 @@ export class AutomationEngine implements IAutomationService { const dialect = isEnvelope ? (expression as { dialect?: string }).dialect : undefined; const exprStr = typeof expression === 'string' ? expression : ((expression as { source?: string })?.source ?? ''); - if (isEnvelope && dialect && dialect !== 'cel' && dialect !== 'flow' && dialect !== 'template') { + if (dialect === 'template' || dialect === 'flow') { + // [#19939] The legacy `{var}` template dialect is deleted (see the + // doc comment): the same set `registerFlow` refuses at every + // condition slot, refused here for a caller that skipped it. + throw new Error( + `condition evaluation error: a condition is CEL, and the \`${dialect}\` dialect is not evaluated — ` + + `source: \`${exprStr}\`. Write the predicate as CEL without \`{…}\` template braces ` + + `(e.g. \`record.status == 'open'\`), as bare text or as \`{ dialect: 'cel', source }\`.`, + ); + } + if (isEnvelope && dialect && dialect !== 'cel') { // Other dialects (cron, js) are not boolean predicates here. return false; } @@ -12413,220 +12363,34 @@ export class AutomationEngine implements IAutomationService { // an unauthored branch must not open. if (exprStr.trim() === '') return false; - // The dialect decision (see the doc comment). An explicit `template`/`flow` - // envelope takes the author at their word; a bare string is sniffed for a - // `{var}` hole; everything else — including an envelope with no dialect — - // is CEL. - const holes = templateHoles(exprStr); - const declaredTemplate = isEnvelope && (dialect === 'template' || dialect === 'flow'); - const useTemplateDialect = declaredTemplate || (!isEnvelope && holes.length > 0); - - // CEL path — bind `vars` scope for `{step.result}` style references via - // the equivalent `vars.step.result` CEL identifier path. - if (!useTemplateDialect) { - try { - const result = ExpressionEngine.evaluate( - { dialect: 'cel', source: exprStr }, - this.celScope(variables, context), - ); - // ADR-0032 §Decision 1c — NO silent fallback. A non-`ok` result is a - // real fault (malformed predicate, or — pre build-validation — a - // `{…}` template mistakenly written into a CEL condition). Surfacing - // it as a thrown, attributed error makes execute()'s catch record a - // loud flow failure, instead of the old `return false` that made a - // broken condition indistinguishable from "condition not met" (#1491). - if (!result.ok) { - throw new Error( - `condition failed to evaluate as CEL: ${result.error?.message ?? 'unknown error'} — ` + - `source: \`${exprStr}\`. Conditions are bare CEL (e.g. \`record.rating >= 4\`); ` + - `do not wrap field references in \`{…}\` template braces.`, - ); - } - return Boolean(result.value); - } catch (err) { - // Re-throw with the source attached (ADR-0032 §1d — errors written - // for self-correction). Never swallow to `false`. - const msg = (err as Error)?.message ?? String(err); + // Bind the `vars` scope, so a `$`-named variable or a node output's + // flat dotted key is read as `vars["…"]`. + try { + const result = ExpressionEngine.evaluate( + { dialect: 'cel', source: exprStr }, + this.celScope(variables, context), + ); + // ADR-0032 §Decision 1c — NO silent fallback. A non-`ok` result is a + // real fault (malformed predicate, or — pre build-validation — a + // `{…}` template mistakenly written into a CEL condition). Surfacing + // it as a thrown, attributed error makes execute()'s catch record a + // loud flow failure, instead of the old `return false` that made a + // broken condition indistinguishable from "condition not met" (#1491). + if (!result.ok) { throw new Error( - msg.includes('source:') ? msg : `condition evaluation error: ${msg} — source: \`${exprStr}\``, + `condition failed to evaluate as CEL: ${result.error?.message ?? 'unknown error'} — ` + + `source: \`${exprStr}\`. Conditions are bare CEL (e.g. \`record.rating >= 4\`); ` + + `do not wrap field references in \`{…}\` template braces.`, ); } - } - - // Legacy template path: {varName} → value, then primitive compare. - let resolved = exprStr; - for (const [key, value] of variables) { - resolved = resolved.split(`{${key}}`).join(String(value)); - } - resolved = resolved.trim(); - - // No `try { … } catch { return false }` around this block (#4347). Nothing - // in it throws — `indexOf` / `slice` / `Number` / `compareValues` are all - // total — so the catch guarded nothing, and the things that CAN throw - // here now are the deliberate refusals below, which a swallow-to-`false` - // would turn straight back into the silent wrong answer they exist to - // prevent (ADR-0032 §1c, same rule as the CEL path above). - - // A hole naming nothing in the variable map is unresolvable (#4336). - // Refuse — see the helper for why `false` was the wrong answer. - this.refuseUnresolvedTemplateHole(exprStr, holes.filter(h => !variables.has(h.slice(1, -1)))); - - // Boolean literals - if (resolved === 'true') return true; - if (resolved === 'false') return false; - - // Comparison operators (ordered by length to match longer operators first) - const operators = ['===', '!==', '>=', '<=', '!=', '==', '>', '<'] as const; - for (const op of operators) { - const idx = resolved.indexOf(op); - if (idx !== -1) { - const left = resolved.slice(0, idx).trim(); - const right = resolved.slice(idx + op.length).trim(); - this.refuseUnresolvedCelOperand(exprStr, left, right); - return this.compareValues(left, op, right); - } - } - - // Numeric truthy check - const numVal = Number(resolved); - if (!isNaN(numVal)) return numVal !== 0; - - // No operator, not a boolean, not a number — this path has no way to - // decide the branch, and `false` used to be its answer (#4336). That made - // a truthy gate on a non-boolean variable — `'{record.status}'`, where - // the value is `'open'` — read as "condition not met" forever, with the - // run still recorded as `success`. Refuse instead, same rule as above. - throw new Error( - `condition evaluation error: \`${resolved}\` is not a predicate — source: \`${exprStr}\`. ` + - `The legacy \`{var}\` template dialect decides a branch by comparing the substituted ` + - `text, so it needs a comparison (\`{status} == 'open'\`) or a value that reads as a ` + - `boolean or number; a bare non-boolean value gives it nothing to compare and used to ` + - `answer \`false\` regardless of the value. Write the predicate as CEL — a condition ` + - `without \`{…}\` braces is evaluated by the CEL engine, where \`record.isActive\` is a ` + - `truthy gate and \`record.status == 'open'\` resolves the field.`, - ); - } - - /** - * Refuse a legacy-dialect condition whose `{…}` holes name no variable - * (#4336). - * - * Substitution replaces the literal text `{}` for each key in the - * variable map, so an unmatched hole survives into the comparison — and the - * template path would then compare the *brace text itself*: - * - * '{lead_record.status} == \'converted\'' - * → '{lead_record.status}' === "'converted'" → always FALSE - * - * The gate never opens, for any record, and the run still reports `success`. - * - * The common way to land here is a **field access on an object variable**: - * `get_record`'s `outputVariable` stores the whole record under one name - * (`lead_record`), so `{lead_record.status}` asks for a key that was never - * written. Note the asymmetry that let this survive — a node's outputs ARE - * flattened into dotted keys (`${node.id}.${key}`), so `{get_lead.id}` - * resolves and looks like proof the spelling works. - * - * CEL resolves that access properly, which is why the prescription is to drop - * the braces rather than to spell the hole differently. - */ - private refuseUnresolvedTemplateHole(source: string, unresolved: readonly string[]): void { - if (unresolved.length === 0) return; - const names = [...new Set(unresolved)]; - throw new Error( - `condition evaluation error: ${names.map(h => `\`${h}\``).join(', ')} did not resolve — ` + - `source: \`${source}\`. The legacy \`{var}\` template dialect substitutes a WHOLE flow ` + - `variable by name, and no variable is named ${names.map(h => `\`${h.slice(1, -1)}\``).join(', ')}. ` + - `Leaving it in place would compare the brace text as a STRING — a branch that is silently ` + - `wrong rather than merely unevaluated — so this is refused. Drop the braces: a condition ` + - `without them is evaluated as CEL, which resolves field access on an object variable ` + - `(\`lead_record.status == 'converted'\`) instead of looking for a variable spelled that way.`, - ); - } - - /** - * Refuse a comparison whose operand is an **unresolved CEL reference** - * (#4347). - * - * The legacy path substitutes `{var}` tokens and then compares whatever text - * is left. A dotted path can never be what that substitution produced — it - * replaces the whole `{…}` token with a VALUE — so a surviving - * `record.amount` means a CEL predicate reached the template path. Comparing - * it as a string is not merely unhelpful, it is silently WRONG *in the - * true direction*: - * - * 'oppRecord.amount > 500000' → 'oppRecord.amount' > '500000' - * → 'o' > '5' → TRUE, for every record - * - * — a gate that reports success while never actually gating. So this refuses - * rather than warns, the same rule ADR-0032 §1c set for the CEL path: a - * predicate that cannot be evaluated is a fault, never a quiet `false` (or, - * here, a quiet `true`). - * - * Since #4336 a *wholly* brace-free condition no longer arrives here at all — - * it is CEL, and `oppRecord.amount > 500000` simply evaluates. What still - * reaches this guard is a dotted reference **mixed into** a template-dialect - * condition (`'{limit} > record.amount'`), where the author is one operand - * away from the right dialect and the string compare would answer anyway. - * - * Only *dotted* references are refused. A bare word compares as a string on - * purpose — `'{status} == active'` is the documented legacy spelling, and - * after substitution both sides are plain words. - */ - private refuseUnresolvedCelOperand(source: string, ...operands: readonly string[]): void { - const offender = operands.find(o => UNRESOLVED_CEL_REFERENCE.test(o)); - if (!offender) return; - throw new Error( - `condition evaluation error: '${offender}' is an unresolved expression reference — ` + - `source: \`${source}\`. This predicate reached the legacy \`{var}\` template path, which ` + - `compares leftover text as STRINGS: a dotted reference is then compared character by ` + - `character (\`record.amount > 500000\` compares 'r' against '5' and is true for every ` + - `record), so the branch would be silently wrong rather than merely unevaluated. ` + - `Write it as CEL — a \`{ dialect: 'cel', source }\` envelope or the \`P\` tagged template ` + - `— which resolves references properly; or, if the \`{var}\` template dialect was meant, ` + - `brace the reference (\`{${offender}}\`).`, - ); - } - - /** - * Compare two string-represented values with an operator. - * - * Quoted operands are unquoted first (#4336). The template dialect compares - * text, so `'{status} == \'active\''` used to substitute to `active == - * 'active'` and compare `active` against `'active'` **with the quotes** — - * never equal, for any value of `status`. That spelling is not exotic: it is - * what the flow docs show for a decision node, and quoting a string literal - * is what every other predicate surface on the platform requires. So the - * quotes are stripped and both the quoted and the bare form (`{status} == - * active`, the older documented spelling) compare the same way. - */ - private compareValues(left: string, op: string, right: string): boolean { - left = unquoteLiteral(left); - right = unquoteLiteral(right); - const lNum = Number(left); - const rNum = Number(right); - const bothNumeric = !isNaN(lNum) && !isNaN(rNum) && left !== '' && right !== ''; - - if (bothNumeric) { - switch (op) { - case '>': return lNum > rNum; - case '<': return lNum < rNum; - case '>=': return lNum >= rNum; - case '<=': return lNum <= rNum; - case '==': case '===': return lNum === rNum; - case '!=': case '!==': return lNum !== rNum; - default: return false; - } - } - // String comparison - switch (op) { - case '==': case '===': return left === right; - case '!=': case '!==': return left !== right; - case '>': return left > right; - case '<': return left < right; - case '>=': return left >= right; - case '<=': return left <= right; - default: return false; + return Boolean(result.value); + } catch (err) { + // Re-throw with the source attached (ADR-0032 §1d — errors written + // for self-correction). Never swallow to `false`. + const msg = (err as Error)?.message ?? String(err); + throw new Error( + msg.includes('source:') ? msg : `condition evaluation error: ${msg} — source: \`${exprStr}\``, + ); } } diff --git a/packages/services/service-automation/src/nested-region-parity.test.ts b/packages/services/service-automation/src/nested-region-parity.test.ts index 502e6d183d6..58166d3cce3 100644 --- a/packages/services/service-automation/src/nested-region-parity.test.ts +++ b/packages/services/service-automation/src/nested-region-parity.test.ts @@ -388,7 +388,7 @@ describe('#4389 — registration validators cover region graphs', () => { }); }); -describe("#4347 — the legacy `{var}` path refuses an unresolved reference", () => { +describe("#4347 — a dotted predicate is CEL, and the legacy `{var}` path is gone (#19939)", () => { let engine: AutomationEngine; beforeEach(() => { engine = new AutomationEngine(silentLogger()); }); @@ -402,15 +402,10 @@ describe("#4347 — the legacy `{var}` path refuses an unresolved reference", () expect(engine.evaluateCondition('oppRecord.amount > 5', vars)).toBe(true); }); - it('still refuses a dotted reference mixed INTO a template-dialect condition', () => { - // A `{…}` hole puts the whole condition in the template dialect, where a - // dotted operand is text and the compare would answer anyway. One operand - // away from the right dialect, so the refusal names the fix. + it('refuses a dotted reference mixed with a `{…}` hole as the brace trap — the template path that compared it as text is deleted', () => { const vars = new Map([['limit', 5]]); - expect(() => engine.evaluateCondition('{limit} > record.amount', vars)).toThrow(/unresolved expression reference/); - expect(() => engine.evaluateCondition('{limit} > record.amount', vars)).toThrow(/dialect: 'cel'/); - // Either side of the operator. - expect(() => engine.evaluateCondition('{limit} == row.shouldRun', vars)).toThrow(/unresolved expression reference/); + expect(() => engine.evaluateCondition('{limit} > record.amount', vars)).toThrow(/do not wrap field references/); + expect(() => engine.evaluateCondition('{limit} == row.shouldRun', vars)).toThrow(/source: `\{limit\} == row\.shouldRun`/); }); it('evaluates the same predicate correctly once it is a CEL envelope', () => { @@ -418,16 +413,18 @@ describe("#4347 — the legacy `{var}` path refuses an unresolved reference", () expect(engine.evaluateCondition({ dialect: 'cel', source: 'oppRecord.amount > 500000' }, vars)).toBe(false); }); - it('leaves the documented legacy dialects alone', () => { + it('the spellings the legacy dialect answered are refused, and their CEL spellings answer the same', () => { const vars = new Map([['amount', 500], ['status', 'active'], ['rate', 1.5]]); - // `{var}` templates: substitution resolves the reference before comparison. - expect(engine.evaluateCondition('{amount} > 100', vars)).toBe(true); + for (const braced of ['{amount} > 100', '{rate} > 1.25', '{status} == active']) { + expect(() => engine.evaluateCondition(braced, vars), braced).toThrow(/do not wrap field references/); + } + expect(engine.evaluateCondition('amount > 100', vars)).toBe(true); // A decimal literal is not a reference. - expect(engine.evaluateCondition('{rate} > 1.25', vars)).toBe(true); - // Bare words compare as strings on purpose — the documented spelling. - expect(engine.evaluateCondition('{status} == active', vars)).toBe(true); + expect(engine.evaluateCondition('rate > 1.25', vars)).toBe(true); + // A string compares quoted. + expect(engine.evaluateCondition("status == 'active'", vars)).toBe(true); expect(engine.evaluateCondition('true', vars)).toBe(true); - // A dotted key that DID resolve is a value by the time the compare runs. - expect(engine.evaluateCondition('{a.b} == 7', new Map([['a.b', 7]]))).toBe(true); + // A node output's flat dotted key is read as the path it spells. + expect(engine.evaluateCondition('a.b == 7', new Map([['a.b', 7]]))).toBe(true); }); }); diff --git a/packages/spec/src/automation/builtin-node-config.zod.ts b/packages/spec/src/automation/builtin-node-config.zod.ts index 7914986b7da..b15a26d588f 100644 --- a/packages/spec/src/automation/builtin-node-config.zod.ts +++ b/packages/spec/src/automation/builtin-node-config.zod.ts @@ -33,14 +33,14 @@ * Live execute-time contracts: each executor `parse()`s its config against * its schema before running (`service-automation`'s `parse-config.ts`), so * type and `required` violations refuse the node as a guard. All of these - * 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), 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`). + * parse the RAW stored config, as authored: no executor interpolates one — + * the single-brace `{token}` dialect is retired from every flow position and + * its interpolator deleted (#19939, ADR-0032 Sequencing 6). The value slots + * refuse a `{token}` (the "value slots" section below), each operand of the + * CRUD `filter` included, where a query placeholder stays the query engine's; + * and 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`). * * ## Unknown keys — closed here too, as of #4001 批 9 * diff --git a/packages/spec/src/automation/flow-template-token.ts b/packages/spec/src/automation/flow-template-token.ts index 0284999c6b6..3d8aff187a9 100644 --- a/packages/spec/src/automation/flow-template-token.ts +++ b/packages/spec/src/automation/flow-template-token.ts @@ -4,21 +4,24 @@ * @module automation/flow-template-token * * **The single-brace `{…}` token grammar of the 17.x flow interpolator** — the - * one reading of it the spec carries, shared by the two judges that retire it: - * `flow-value-slot-template.ts` (the value slots, #19939) and + * judges' frozen reading of a RETIRED dialect, shared by the two judges that + * refuse it: `flow-value-slot-template.ts` (the value slots, #19939) and * `flow-text-slot-template.ts` (the text slots, #22110) — and by the two * contracts that refuse it at a LITERAL position, a `notify` node's * `sourceObject`, `actorId` and `payload` and an `http` node's `method`, * `durable`, `timeoutMs` and `signingSecret` (`io-node-config.zod.ts`, * #19939). * - * Which `{…}` the interpolator substitutes (`/\{([^{}]+)\}/`), and how it - * dispatches a token — date macro, then `$User.`, then a variable path, then - * arithmetic — are the regexes of `resolveToken` in `service-automation`'s - * `builtin/template.ts`, copied here because the spec cannot import a - * runtime. `value-slot-template-grammar.test.ts` in that package drives the - * interpolator over the kept spellings and the refused ones, so the two - * readings cannot drift apart unnoticed. + * Which `{…}` the interpolator substituted (`/\{([^{}]+)\}/`), and how it + * dispatched a token — date macro, then `$User.`, then a variable path, then + * arithmetic — are the regexes of `resolveToken`, which lived in + * `service-automation`'s `builtin/template.ts` until #19939 deleted it with + * every call site (ADR-0032 Sequencing 6). The copy here is FROZEN: nothing + * reads the dialect any more, and this module exists so a refusal can name + * each token's replacement the way the interpolator read that token. It + * changes only with a judge's remedy, never to track a runtime. + * `value-slot-template-grammar.test.ts` in that package pins what each + * remedy evaluates to through the flow's CEL scope. * * ## ⛔ Package-internal — NOT a public export * @@ -33,7 +36,7 @@ * token — or an envelope — at a position the executor reads AS WRITTEN (a * `notify` node's `sourceObject`, `actorId` and `payload`; an `http` node's * `method`, `durable` and `timeoutMs`, and its `headers` map as a whole), - * never through the interpolator. The build doors' value arm (`flow-node-config-refusals.ts`, + * never through an interpolator. The build doors' value arm (`flow-node-config-refusals.ts`, * `builtinValueJudged`) leaves every other token-carrying value alone, since * only the interpolation decides what the run will see there; at a position * read as written the run sees exactly what the build sees, so the @@ -41,7 +44,7 @@ */ export const READ_AS_WRITTEN_PARAM = 'readAsWritten'; -/** The interpolator's token — `interpolateString`'s `/\{([^{}]+)\}/g`. */ +/** The 17.x interpolator's token — `interpolateString`'s `/\{([^{}]+)\}/g`. */ export const TEMPLATE_TOKEN = /\{([^{}]+)\}/g; /** `resolveToken`'s `dateFnMatch`, verbatim: `NOW()` / `TODAY()` with an optional `± N` day offset. */ @@ -62,10 +65,10 @@ const ARITHMETIC_CHARSET = /^[\w\s+\-*/%().,?:<>=!&|"'$]+$/; */ const EXPRESSION_SHAPE = /[+\-*/%<>=!&|?]|[A-Za-z_$][\w$.]*\s*\(/; -/** What the interpolator does with one token — the dispatch order of `resolveToken`. */ +/** What the 17.x interpolator did with one token — the dispatch order of `resolveToken`. */ export type TemplateTokenKind = 'date-macro' | 'user' | 'path' | 'expression' | 'unresolvable'; -/** Classify one token's inner text the way `resolveToken` dispatches it. */ +/** Classify one token's inner text the way `resolveToken` dispatched it. */ export function templateTokenKind(inner: string): TemplateTokenKind { const trimmed = inner.trim(); if (!trimmed) return 'unresolvable'; @@ -82,13 +85,13 @@ export interface TemplateToken { readonly text: string; /** The text between the braces, trimmed. */ readonly inner: string; - /** How the interpolator dispatches it. */ + /** How the 17.x interpolator dispatched it. */ readonly kind: TemplateTokenKind; /** Offset of {@link text} in the string. */ readonly index: number; } -/** Every `{…}` token the interpolator would substitute in `value`, in order. */ +/** Every `{…}` token the 17.x interpolator substituted in `value`, in order. */ export function templateTokensOf(value: string): TemplateToken[] { const out: TemplateToken[] = []; for (const match of value.matchAll(TEMPLATE_TOKEN)) { diff --git a/packages/spec/src/automation/flow-text-slot-template.ts b/packages/spec/src/automation/flow-text-slot-template.ts index 309393c9dc9..3bc4d46552b 100644 --- a/packages/spec/src/automation/flow-text-slot-template.ts +++ b/packages/spec/src/automation/flow-text-slot-template.ts @@ -181,17 +181,16 @@ export interface FlowNodeTextSlot { * renderer; and since #19939 `http-nodes.ts` renders `url` and each `headers` * value — the 17.x contract, parsed after the interpolation, refused * anything but a string there, so a whole token never handed a typed value - * on either). Every OTHER string the flow - * interpolator reads keeps its single-brace dialect: a value-like position - * whose single token hands its resolved value over with its TYPE (a - * `filter`) is not a text template, and the value slots (`fields.*`, - * `assignments.*`, the callee maps `subflow.input.*`, `map.input.*`, - * `script.inputs.*`, a screen's `defaults.*`, field `defaultValue` and - * `recordId`, the `loop` / `map` `collection`, a notify node's `recipients`, - * `templateData.*` and `sourceId`, and an http node's `body`) are CEL's - * (#19939), and a notify node's `sourceObject`, `actorId` and `payload` and an - * http node's `method`, `durable`, `timeoutMs` and `signingSecret` are - * literals, read as written (#19939). + * on either). Every OTHER position the 17.x flow interpolator read is no + * text template: the value slots (`fields.*`, `assignments.*`, the callee + * maps `subflow.input.*`, `map.input.*`, `script.inputs.*`, a screen's + * `defaults.*`, field `defaultValue` and `recordId`, the `loop` / `map` + * `collection`, a notify node's `recipients`, `templateData.*` and + * `sourceId`, an http node's `body`, and each operand of a CRUD node's + * `filter`) are CEL's (#19939), and a notify node's `sourceObject`, `actorId` + * and `payload` and an http node's `method`, `durable`, `timeoutMs` and + * `signingSecret` are literals, read as written (#19939). The interpolator + * itself is deleted (#19939, ADR-0032 Sequencing 6). */ export const FLOW_NODE_TEXT_SLOTS: readonly FlowNodeTextSlot[] = [ { nodeType: 'notify', key: 'title', label: 'notify title' }, diff --git a/packages/spec/src/automation/flow.zod.ts b/packages/spec/src/automation/flow.zod.ts index 226a5605c97..41a55ac0d61 100644 --- a/packages/spec/src/automation/flow.zod.ts +++ b/packages/spec/src/automation/flow.zod.ts @@ -980,8 +980,10 @@ export const FlowEdgeSchema = lazySchema(() => strictObject( * { 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: { dialect: "cel", source: "record.id" } }, fields: { status: "approved" } } }, + * // `connectorConfig.input` is handed to the action as written: no template + * // is read there, so it holds literals. * { id: "submit_for_approval", type: "connector_action", label: "Submit", position: {x: 100, y: 200}, - * connectorConfig: { connectorId: "approvals_desk", actionId: "submit", input: { orderId: "{record.id}" } } } + * connectorConfig: { connectorId: "approvals_desk", actionId: "submit", input: { queue: "large_orders" } } } * ], * edges: [ * { id: "e1", source: "start", target: "check_amount" }, diff --git a/packages/spec/src/data/context-tokens.zod.ts b/packages/spec/src/data/context-tokens.zod.ts index 806b358a92b..eec2fad5a66 100644 --- a/packages/spec/src/data/context-tokens.zod.ts +++ b/packages/spec/src/data/context-tokens.zod.ts @@ -106,10 +106,10 @@ import { DATE_MACRO_WRAPPED_RE, isDateMacroToken } from './date-macros.zod.js'; * * - `{record_id}` — this filter token. A whole filter VALUE, resolved to the * id of the record the page shows. - * - `{recordId}` — a URL / flow-template placeholder: an action's - * `newTabUrl` (`action.zod.ts`) and the flow template dialect - * (`@objectstack/lint`'s `flow-template-grammar.ts`). It is interpolated by - * those surfaces' own template engines and is NOT a filter token. Written + * - `{recordId}` — a URL placeholder: an action's `newTabUrl` + * (`action.zod.ts`), interpolated by that surface's own template engine, + * and a variable's spelling in the retired flow template dialect (refused + * at every flow position since #19939). It is NOT a filter token. Written * inside a filter it is refused, with `{record_id}` suggested. * - `'record_id'` — a page-variable TYPE (`PageVariableSchema.type` in * `page.zod.ts`), naming what a page variable holds, e.g. the selection of @@ -285,9 +285,8 @@ export const CONTEXT_TOKEN_DESCRIPTIONS: Record = { * still see and act on it. * * The `record_id` rows follow the same rule. `recordid` is `{recordId}` - * lower-cased, and `{recordId}` is the URL / flow-template placeholder - * (`newTabUrl`, the flow template dialect), which is a correct spelling - * somewhere else. `record.id` is the flow template dialect's path to its + * lower-cased, and `{recordId}` is the URL placeholder (`newTabUrl`), which + * is a correct spelling somewhere else. `record.id` is a flow's path to its * triggering record. `record-id` is the kebab shape #5586 recognised, and * `current_record_id` is the `current_*` shape the session tokens teach. */ diff --git a/packages/spec/src/migrations/registry.ts.template b/packages/spec/src/migrations/registry.ts.template index 2b9b0cc1be6..b4a5bafb61a 100644 --- a/packages/spec/src/migrations/registry.ts.template +++ b/packages/spec/src/migrations/registry.ts.template @@ -2017,8 +2017,9 @@ const STEP18_RATIONALE: readonly RationaleFragment[] = [ + '(JSON-quoted against ISO text), and a whole-slot object differently in a screen or `end` text or a ' + 'notify link, and an `http` url or header that was one token holding anything but a string was refused ' + 'after the interpolation where the engine renders it as text, so ' - + 'the rewrite is the author\'s to check. Every other flow string keeps the single-brace dialect. Its ' - + 'D3 record is the semantic entry `flow-text-slot-single-brace-refused`.', + + 'the rewrite is the author\'s to check. No other flow string reads the single-brace dialect either: ' + + 'the value slots take a CEL value envelope, a node\'s literals are read as written, and the 17.x ' + + 'interpolator is deleted. Its D3 record is the semantic entry `flow-text-slot-single-brace-refused`.', }, { id: 'flow-text-slot-unbound-dollar-root-refused', @@ -2039,10 +2040,18 @@ const STEP18_RATIONALE: readonly RationaleFragment[] = [ text: 'It retires the single-brace `{…}` template dialect from the flow VALUE slots (the C half of the ' + 'maintainer\'s ruling D on the flow expression dialects): the `assignment` node\'s values, in all ' - + 'three shapes, and the `fields` map of `create_record` and `update_record`, where a CEL value ' - + 'envelope is already the expression form. A string there is now the literal ' - + 'text it spells, and one carrying a `{…}` token is refused — by `FlowValueSlotSchema`, ' - + '`registerFlow`, `objectstack validate` and the executor alike — with the CEL spelling of each token. ' + + 'three shapes; the `fields` map of `create_record` and `update_record`; the maps a node hands to a ' + + 'callee (a `subflow`\'s and a `map`\'s `input`, a `script`\'s `inputs`); a `screen`\'s `defaults`, each ' + + 'field\'s `defaultValue` and its `recordId`; the `collection` a `loop` with a `body` and a `map` ' + + 'iterate; a `notify` node\'s `recipients`, `templateData` and `sourceId`; an `http` node\'s `body`; and ' + + 'each operand of a `get_record`, `update_record` or `delete_record` `filter`. A CEL value envelope is ' + + 'the expression form in each. A string there is now the literal text it spells (a `collection`\'s ' + + 'literal is an inline array), and one carrying a `{…}` token is refused — by the node contracts, ' + + '`registerFlow`, `objectstack validate` and the executor alike — with the CEL spelling of each token; ' + + 'a whole-string query placeholder at a filter operand (`{current_user_id}`, `{today}`) stays the ' + + 'query engine\'s. A `notify` node\'s `sourceObject`, `actorId` and `payload` and an `http` node\'s ' + + '`method`, `durable`, `timeoutMs` and `signingSecret` are literals read as written, where a token is ' + + 'refused too. ' + 'No D2 conversion exists: every authored spelling was measured lossy (an absent key writes nothing ' + 'under the template and fails under CEL; CEL divides two integers as integers), so which value an ' + 'absent key should write is the author\'s judgment. The `$User` paths are refused too: the flow CEL ' diff --git a/packages/verify/src/handle.automation-door.test.ts b/packages/verify/src/handle.automation-door.test.ts index 1e39b307b47..e7849d653c0 100644 --- a/packages/verify/src/handle.automation-door.test.ts +++ b/packages/verify/src/handle.automation-door.test.ts @@ -150,19 +150,22 @@ describe('stack.automation.evaluateCondition (#22301 item 8)', () => { } }); - it('never rewrites the condition: a bare `{var}` string keeps the template dialect the engine gives it', async () => { + it('never rewrites the condition: a bare `{var}` string reaches the engine as written, and is refused as the engine refuses it', async () => { const holed = '{amount} > 100'; const spy = vi.spyOn(engine, 'evaluateCondition'); - // The engine reads a bare string with a `{…}` hole as the template dialect. - expect(await stack.automation.evaluateCondition(holed, { amount: 5000 })).toBe(true); - expect(spy.mock.calls[0][0]).toBe(holed); - - // The same text in a CEL envelope is the brace trap, and the engine refuses it. - const { door, direct } = await both({ dialect: 'cel', source: holed }, { amount: 5000 }); - expect(direct.fault).toBeInstanceOf(Error); - expect(door.fault).toBeInstanceOf(Error); - expect((door.fault as Error).message).toBe((direct.fault as Error).message); + // The engine reads every condition as CEL — the legacy `{var}` template + // dialect is deleted (#19939) — so a `{…}` hole is the brace trap, bare + // or in a CEL envelope, and the door hands the engine's refusal back. + for (const condition of [holed, { dialect: 'cel', source: holed }]) { + spy.mockClear(); + const { door, direct } = await both(condition, { amount: 5000 }); + expect(spy.mock.calls[0][0]).toBe(condition); + expect(direct.fault).toBeInstanceOf(Error); + expect(door.fault).toBeInstanceOf(Error); + expect((door.fault as Error).message).toBe((direct.fault as Error).message); + expect((door.fault as Error).message).toContain('do not wrap field references'); + } }); it("rejects with the engine's own error when the condition faults, never `false`", async () => { diff --git a/packages/verify/src/handle.ts b/packages/verify/src/handle.ts index 9f3fd264f2b..f6a3e62a44a 100644 --- a/packages/verify/src/handle.ts +++ b/packages/verify/src/handle.ts @@ -404,13 +404,14 @@ export interface VerifyHandle { * * `condition` is handed to the engine as given, in any shape it takes: an * envelope (`{ dialect: 'cel', source }`) or a string, which the engine - * reads as CEL unless it carries a `{var}` hole (the legacy template - * dialect). The handle never rewrites it. The engine's own flow sites - * hand it an envelope: a start node's `condition` and a `decision` - * branch's `expression` are wrapped as `{ dialect: 'cel', source }`, and - * an edge's condition already is one. So pass the envelope to model any - * of them; a bare string gets the reading a screen field's `visibleWhen` - * gets. + * reads as CEL — a `{var}` hole is the brace trap the engine refuses, and + * a `template` or `flow` envelope is refused too, since the legacy + * template dialect is deleted (#19939). The handle never rewrites it. The + * engine's own flow sites hand it an envelope: a start node's `condition` + * and a `decision` branch's `expression` are wrapped as + * `{ dialect: 'cel', source }`, and an edge's condition already is one. So + * pass the envelope to model any of them; a bare string gets the reading + * a screen field's `visibleWhen` gets. * * `variables` is a plain object, and each own key becomes one entry of * the `Map` the engine takes: `{ record, previous }` binds `record` and